Skip to content
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 AEPsych models to not require dim instead of lb, ub in constructor #379

Open
crasanders opened this issue Sep 8, 2024 · 1 comment

Comments

@crasanders
Copy link
Contributor

AEPsych models currently require lower bounds and upper bounds of parameters to be passed into their constructors, but the model itself should not have to worry about the bounds of the space; that's the responsibility of the strategy. All the model really needs to know is the dimensionality of the space. Removing the lb and ub from the constructor would reduce a lot of boilerplate. Wherever a model method requires self.lb or self.ub, we should be able to refactor it so that the method takes those as parameters that can be passed in by the strategy.

@JasonKChow
Copy link
Contributor

ub/lb/dim arguments in the [common] block are all being deprecated.

ub/lb will no longer be defined in the [common] block, instead they will be defined with upper_bound/lower_bound in parameter specific blocks.

dim should never be defined in config files anymore and should be inferred based on all of the parameters. This also implies parnames must always be set now.

Important places to check their usage:

  • Accessing ub/lb to build objects from the config currently rely on ub/lb in [common] as fallback, this needs to be changed to build ub/lb tensors from parameter-specific settings
  • Using dim from the config is used to setup benchmarking test cases, this should be done with benchmarking specific behavior instead. A good indicator of this are in test configs that do not have parnames set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants