[Refactor] How parameters are represented in HSSM#590
Merged
Conversation
This was
linked to
issues
Oct 5, 2024
Closed
cpaniaguam
requested changes
Oct 7, 2024
Collaborator
cpaniaguam
left a comment
There was a problem hiding this comment.
Thanks for this redesign! Added some suggestions and a question about default_model_config.
Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>
Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>
…to param-refactor
Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>
cpaniaguam
reviewed
Oct 15, 2024
Collaborator
cpaniaguam
left a comment
There was a problem hiding this comment.
Couple more observations.
Member
AlexanderFengler
left a comment
There was a problem hiding this comment.
This looks in good shape!
cpaniaguam
approved these changes
Oct 28, 2024
Collaborator
cpaniaguam
left a comment
There was a problem hiding this comment.
Approving but left some comments for your considerations about the parse_bambi method. I think it's doing many things that are a bit too intertwined.
Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>
…to param-refactor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces a refactored
Paramclass to represent parameter specifications in HSSM.Before the refactor,
Paramwas a single class that both accepts user inputs and performs internal processing, such as merging with default values and truncating priors, which made the code in the class very messy. Some of the functionalities also bled into the HSSM class, which bloated the class as well. The refactor aims to separate the concerns of the oldParamclass into multiple classes, each taking one responsibility of the Param class. This completely moves the concerns out of the HSSM class, making the main class much less bloated (even though there is still room for improvement).The improvements are:
UserParamclass, which is exported ashssm.Param, for storing user input. (closes [Refactor]UserParamclass only for user class specification #564)Paramclass that defines some common functionalities for the parameter representation. (closes [Refactor]Paramclass, which is a base class forSimpleParamandRegressionParamclass #560)Paramclass is theSimpleParamclass, which handles the situation where the parameter is not a regression. Its subclassDefaultParamhandles the situation where the prior is not specified in the non-regression case (where we do not truncate the prior). (closes [Refactor]SimpleParamclass #561)RegressionParamclass for handling the regression case (closes [Refactor]RegressionParamclass #562)Paramsclass for creating these params both from user specification, or defaults, or both. It also takes away some of the responsibilities of the HSSM class (closes [Refactor]Paramsclass, which wraps around a dictionary and stores allParaminformation #559)Tests for all these new classes are under
tests/paramdirectory.The refactor also removes "hierarchical" argument to
HSSM, addingglobal_formulainstead. (closes #431, #432)Some other changes to other code files are necessary for this gigantic refactor to work, but most of the changes are under
src/hssm/param.