Skip to content

Handle configs via dependency injection#931

Merged
cpaniaguam merged 73 commits intorlssm-class-make-model-distfrom
930-pass-configs-via-dependency-injection-into-model-classes-basemodelconfig-only-dict-supported
Mar 31, 2026
Merged

Handle configs via dependency injection#931
cpaniaguam merged 73 commits intorlssm-class-make-model-distfrom
930-pass-configs-via-dependency-injection-into-model-classes-basemodelconfig-only-dict-supported

Conversation

@cpaniaguam
Copy link
Copy Markdown
Collaborator

@cpaniaguam cpaniaguam commented Mar 11, 2026

This pull request refactors the model configuration and initialization logic for the HSSM base class, moving the responsibility for constructing and validating model configurations out of HSSMBase and into the configuration classes themselves. This results in a cleaner, more modular design and simplifies subclassing and serialization. The most important changes are grouped below.

Refactoring and Simplification of Model Configuration

  • The HSSMBase class now requires a fully initialized BaseModelConfig (usually a Config) to be passed in, rather than accepting a mix of model names, config dicts, and other parameters. All likelihood, parameter, and data information is now drawn from this object, and the constructor no longer builds the config itself. [1] [2] [3]
  • The logic for building and validating a model config from defaults, user input, and external sources (like ssms-simulators) has been moved to a new _build_model_config classmethod on Config. This method is now responsible for all config normalization and validation.
  • The _build_model_config and get_config_class methods have been removed from HSSMBase and BaseModelConfig, and the responsibility for config class resolution is now handled by the config classes themselves. [1] [2] [3] [4]

Improved Initialization Argument Capture and Serialization

  • The mechanism for capturing constructor arguments for save/load serialization has been improved. A new static method _store_init_args provides a robust way for subclasses to snapshot their initialization arguments, and a default empty snapshot is set in the base class to prevent serialization errors if subclasses forget to call it. [1] [2] [3]
  • The __getstate__ method now raises a clear error if the initialization snapshot is missing, making the contract for serialization explicit and robust.

API and Documentation Updates

  • The constructor signature and docstrings for HSSMBase have been updated to reflect the new configuration workflow, clarifying that a BaseModelConfig is required and describing its role. [1] [2]
  • Convenience properties for the number of parameters and extra fields have been added to BaseModelConfig and cleaned up in RLSSMConfig. [1] [2]

Internal Imports and Logging

  • Imports related to model configuration have been cleaned up and moved to the appropriate files. Logging for model configuration choices has been centralized in config.py. [1] [2] [3]

@cpaniaguam cpaniaguam changed the base branch from main to rlssm-class-make-model-dist March 11, 2026 18:08
@cpaniaguam cpaniaguam requested a review from Copilot March 11, 2026 18:12
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors HSSM/RLSSM configuration handling to use typed config objects built via factory methods (dependency injection into HSSMBase), and updates RLSSM internals/tests to align with the new config and attribute patterns.

Changes:

  • Refactors HSSMBase to accept a pre-built BaseModelConfig and adds deprecated proxy properties for legacy attributes.
  • Adds Config._build_model_config(...) and uses it in HSSM.__init__; RLSSM now builds a validated Config via RLSSMConfig._build_model_config(...).
  • Updates RLSSM public attributes/tests (n_participants, n_trials) and adjusts distribution construction to work with the new config flow.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/test_rlssm.py Updates tests to use new RLSSM public attributes.
src/hssm/rl/rlssm.py Removes config mixin inheritance; injects validated config into HSSMBase.
src/hssm/hssm.py Adds HSSM.__init__ that builds a validated Config via factory then calls HSSMBase.
src/hssm/config.py Introduces centralized config factory (Config._build_model_config) and RLSSM helper factory.
src/hssm/base.py Changes base initializer to consume injected config and adds deprecated proxy properties + init-arg capture helper.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

src/hssm/base.py Outdated
Comment on lines +387 to +391
warnings.warn(
f"`{name}` is deprecated; use `self.model_config.{name}` instead.",
DeprecationWarning,
stacklevel=2,
)
Copy link

Copilot AI Mar 11, 2026

Choose a reason for hiding this comment

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

The new deprecated proxy properties always emit DeprecationWarning via _deprecation_warn(). Since core internals (including HSSMBase.__init__) call self.list_params, self.choices, etc., this will trigger warnings during normal model construction even when users never touch deprecated attributes (and can break in environments that treat warnings as errors). Consider having internal code access self.model_config.* directly, or gating warnings so they only fire for external/user-level access.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

src/hssm/base.py:276

  • After introducing deprecated proxy properties (e.g., self.list_params, self.choices), this initializer still uses them internally (_validate_choices(), if self.list_params is None, len(self.choices)). That will emit DeprecationWarnings during normal model construction (especially under warnings-as-errors). Prefer direct access to the authoritative model_config here (e.g., model_config.list_params, model_config.choices).
        self._validate_choices()

        # region Avoid mypy error later (None.append). Should list_params be Optional?
        if self.list_params is None:
            raise ValueError(

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

…Config instance, preserving the original configuration.
…asses-basemodelconfig-only-dict-supported' into inject-RLSSMConfig-directly-into-HSSMBase
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.

ok to go for me.

@cpaniaguam cpaniaguam marked this pull request as ready for review March 31, 2026 15:51
@cpaniaguam cpaniaguam merged commit b0e2179 into rlssm-class-make-model-dist Mar 31, 2026
4 checks passed
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

Successfully merging this pull request may close these issues.

Pass configs via dependency injection into model classes (BaseModelConfig-only; dict supported)

4 participants