Regularization API for preference comparisons #481
Conversation
Co-authored-by: Adam Gleave <adam@gleave.me>
… EnsembleRewardNet initalizer
…ng before this ...
…gs to predict processed
… default when using a reward ensemble with preference comparison it will wrap it in a add std wrapper.
…dic processed rather than predict_th.
…t the initalizations of the ensemble members are different.
…edict processed. RewardNetWrapper now implments predict_processed to ensure this is the default behavour.
There was a problem hiding this comment.
Factory approach seems good! It's an easy to use API. The actual implementation is a little trickier to follow over direct instantiation. But overall I think it's the least-bad option.
The main alternative would be to just make the callee to preference comparisons responsible for passing in a non-empty custom_logger and create the optimizer. This isn't crazy -- the scripts are already creating a custom logger, and making the callee create the optimizer does at least give flexibility as to what optimization class to use. But it seems like it's adding a lot of friction for people who want to just programatically use the Python API, whereas the regularization factory is easy to use.
Another approach would have been to just let you instantiate the regularizer without specifying optimizer or logger, and have this be something that's set after instantiation (with some check that they have been set before calling .regularize()). But this seems more error prone: it's nice to have instantiation mean the object is actually ready to use.
There's a few small comments outstanding (e.g. adding tests), but once those addressed I think we should be good to go -- just request a re-review once it's ready to look at again.
…dynamic-l2-regularization
Co-authored-by: Adam Gleave <adam@gleave.me>
Co-authored-by: Adam Gleave <adam@gleave.me>
|
Added all the requested changes, except the tests for the |
Fixes #461.