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

Keep _process_priors() and _unload_params() in CLVModel #182

Closed
ColtAllen opened this issue Mar 1, 2023 · 2 comments
Closed

Keep _process_priors() and _unload_params() in CLVModel #182

ColtAllen opened this issue Mar 1, 2023 · 2 comments

Comments

@ColtAllen
Copy link
Collaborator

Currently, _process_priors() and _unload_params() are being defined in every CLV model, but the only differences are the specific prior/param names. If we add both methods to the base CLVModel as generic loop operators, and a list attribute in the child model classes containing the param names, it would eliminate a lot of redundant code when adding new models. #133 partially resolves _process_priors().

Also, in each child model class every predictive method calls _unload_params() and clv.utils.to_xarray(). It may be worthwhile to write a decorator in CLVModel for predictive methods that handles this automatically, but I need to look into this more.

@ColtAllen
Copy link
Collaborator Author

I started working on this, but I'm not sure if these code changes can accommodate covariates, so I'll return to this after progress has been made on #134.

@ColtAllen ColtAllen mentioned this issue May 3, 2023
6 tasks
@ColtAllen
Copy link
Collaborator Author

I'm currently adding support for covariates to ParetoNBDModel, which creates nuances to _unload_params that are not agnostic across all models. _process_priors is also in the base class now, so I'm closing this.

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

No branches or pull requests

1 participant