Skip to content

[Refactor] How parameters are represented in HSSM#590

Merged
digicosmos86 merged 44 commits into
mainfrom
param-refactor
Oct 28, 2024
Merged

[Refactor] How parameters are represented in HSSM#590
digicosmos86 merged 44 commits into
mainfrom
param-refactor

Conversation

@digicosmos86
Copy link
Copy Markdown
Collaborator

This PR introduces a refactored Param class to represent parameter specifications in HSSM.

Before the refactor, Param was 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 old Param class 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:

  1. A user-facing UserParam class, which is exported as hssm.Param, for storing user input. (closes [Refactor] UserParam class only for user class specification #564)
  2. An internal Param class that defines some common functionalities for the parameter representation. (closes [Refactor] Param class, which is a base class for SimpleParam and RegressionParam class #560)
  3. Inheriting the Param class is the SimpleParam class, which handles the situation where the parameter is not a regression. Its subclass DefaultParam handles the situation where the prior is not specified in the non-regression case (where we do not truncate the prior). (closes [Refactor] SimpleParam class #561)
  4. The RegressionParam class for handling the regression case (closes [Refactor] RegressionParam class #562)
  5. A dict-like Params class 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] Params class, which wraps around a dictionary and stores all Param information #559)

Tests for all these new classes are under tests/param directory.

The refactor also removes "hierarchical" argument to HSSM, adding global_formula instead. (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.

Copy link
Copy Markdown
Collaborator

@cpaniaguam cpaniaguam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this redesign! Added some suggestions and a question about default_model_config.

Comment thread src/hssm/hssm.py Outdated
Comment thread src/hssm/hssm.py
Comment thread src/hssm/hssm.py Outdated
Comment thread src/hssm/param/params.py
Comment thread tests/test_hssm.py Outdated
Comment thread src/hssm/param/regression_param.py
Comment thread src/hssm/param/regression_param.py Outdated
Comment thread src/hssm/param/params.py
Comment thread src/hssm/param/params.py
Comment thread src/hssm/param/params.py
digicosmos86 and others added 4 commits October 7, 2024 14:38
Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>
Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>
Co-authored-by: Carlos Paniagua <cpaniaguam@gmail.com>
Copy link
Copy Markdown
Collaborator

@cpaniaguam cpaniaguam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple more observations.

Comment thread src/hssm/hssm.py Outdated
Comment thread src/hssm/param/regression_param.py
Comment thread src/hssm/param/regression_param.py
Comment thread tests/param/test_params.py
Copy link
Copy Markdown
Member

@AlexanderFengler AlexanderFengler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks in good shape!

Comment thread src/hssm/config.py
Comment thread src/hssm/hssm.py
Comment thread src/hssm/param/param.py
Comment thread src/hssm/param/params.py
Comment thread src/hssm/param/params.py Outdated
Comment thread src/hssm/param/utils.py
Copy link
Copy Markdown
Member

@AlexanderFengler AlexanderFengler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Copy Markdown
Collaborator

@cpaniaguam cpaniaguam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/hssm/param/params.py
Comment thread src/hssm/param/params.py
Comment thread src/hssm/param/params.py Outdated
Comment thread src/hssm/param/params.py
Comment thread src/hssm/param/params.py
Comment thread src/hssm/param/regression_param.py
@digicosmos86 digicosmos86 merged commit 478fc5c into main Oct 28, 2024
@digicosmos86 digicosmos86 deleted the param-refactor branch February 10, 2026 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment