-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor MAB and Strategy Classes with Cold Start Methods and Enhanced Validation #35
Conversation
0748967
to
29723e1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This set of changes are breaking, so should also include a version increase in the tomlfile :)
Some comments (for nitpicking) are written only for the first instance for some repeating events
@adarmiento Thank you for the thorough inspection of the code! Much appreciated! |
We will release a major version after this PR is merged 🙏 |
fb27dcf
to
61a8c49
Compare
61a8c49
to
576a258
Compare
576a258
to
5bcb9b5
Compare
aa4ff23
to
073821c
Compare
ed27032
to
da916fa
Compare
847158b
to
9404315
Compare
f092097
to
bd08425
Compare
bd08425
to
42c65b7
Compare
actions: Dict[ActionId, Model], | ||
epsilon: Optional[Float01] = None, | ||
default_action: Optional[ActionId] = None, | ||
**strategy_kwargs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add docstring
@@ -73,32 +118,44 @@ class BestActionIdentification(Strategy): | |||
|
|||
Parameters | |||
---------- | |||
exploit_p: Float_0_1 (default=0.5) | |||
exploit_p: Optional[Float01], 0.5 if not specified |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's call this class BestActionIdentificationBandit to keep it homogeneous with the others
ed5a5a2
to
eb3dbef
Compare
…d Validation Change log: 1. Moved Strategy, Model, and MAB to strategy.py, model.py, and to the new mab.py. base.py is now only for definitions and abstract PyBanditsBaseModel. The abstract MAB now allows for all childs to either accept strategy instance as parameter, or to get the strategy parameters and instantiate correspondingly. 2. The from_state functionality is now directly inherited by all MABs from BaseMab. 3. Replaced all cold_start methods in cmab.py and smab.py with cold_start stemming from BaseMab. Correspondingly, updated test cases to use the new cold_start_instantiate methods. 4. Introduced numerize_field and get_expected_value_from_state methods in the Strategy class to handle default values and state extraction. Added field_validator for exploit_p in BestActionIdentification and subsidy_factor in CostControlBandit to ensure proper default handling and validation. 5. Merged common functionality into a new CostControlStrategy abstract class, which is now inherited by CostControlBandit and MultiObjectiveCostControlBandit. Simplified the select_action methods by using helper methods like _evaluate_and_select and _reduce. 6. Plugged get_pareto_front into a new MultiObjectiveStrategy abstract class, which is now inherited by MultiObjectiveBandit and MultiObjectiveCostControlBandit. 7. In model.py. Removed the redundant BaseBetaMO and BaseBayesianLogisticRegression. Added cold_start_instantiate method to BetaMO and BayesianLogisticRegression models. 8. Added extract_argument_names_from_function under utils.py to allow extract function parameter names by handle. 9. Changed test_base.py into test_mab.py. 10. Updated deprecated linter settings in pyproject.toml. 11. Added test_smab_mo_cc_update test on test_smab.py. 12. Changed version to 1.0.0 on pyproject.toml.
eb3dbef
to
c965c27
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good changes in the library @Shahar-Bar !
…d Validation (#63) Refactor MAB and Strategy Classes with Cold Start Methods and Enhanced Validation (#35) ### Changes * Moved Strategy, Model, and MAB to strategy.py, model.py, and to the new mab.py. base.py is now only for definitions and abstract PyBanditsBaseModel. The abstract MAB now allows for all childs to either accept strategy instance as parameter, or to get the strategy parameters and instantiate correspondingly. * The from_state functionality is now directly inherited by all MABs from BaseMab. * Replaced all cold_start methods in cmab.py and smab.py with cold_start stemming from BaseMab. Correspondingly, updated test cases to use the new cold_start_instantiate methods. * Introduced numerize_field and get_expected_value_from_state methods in the Strategy class to handle default values and state extraction. Added field_validator for exploit_p in BestActionIdentification and subsidy_factor in CostControlBandit to ensure proper default handling and validation. * Merged common functionality into a new CostControlStrategy abstract class, which is now inherited by CostControlBandit and MultiObjectiveCostControlBandit. Simplified the select_action methods by using helper methods like _evaluate_and_select and _reduce. * Plugged get_pareto_front into a new MultiObjectiveStrategy abstract class, which is now inherited by MultiObjectiveBandit and MultiObjectiveCostControlBandit. * In model.py. Removed the redundant BaseBetaMO and BaseBayesianLogisticRegression. Added cold_start_instantiate method to BetaMO and BayesianLogisticRegression models. * Added extract_argument_names_from_function under utils.py to allow extract function parameter names by handle. * Changed test_base.py into test_mab.py. * Updated deprecated linter settings in pyproject.toml. * Added test_smab_mo_cc_update test on test_smab.py. * Changed version to 1.0.0 on pyproject.toml.
Refactor MAB and Strategy Classes with Cold Start Methods and Enhanced Validation
Change log: