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

Submodels take kwargs variable splits rather than NamedTuples. #65

Conversation

SamuelBrand1
Copy link
Collaborator

@SamuelBrand1 SamuelBrand1 commented Feb 23, 2024

Goal

This PR is aimed at addressing the problem outlines in #63 .

API changes

None at top level API (e.g. running this script.

Where NamedTuples are used at lower level we can splat their fields as kwargs e.g.

#Predictive distribution of ascerted cases
@submodel generated_y_t, generated_y_t_aux = observation_process_obj.observation_model(
y_t,
I_t,
epimodel::AbstractEpiModel;
pos_shift = pos_shift,
observation_process_obj.observation_model_priors...
)

Closes #63

…ng `kwargs` variable splits rather than `NamedTuple`s; updated tests
@SamuelBrand1 SamuelBrand1 linked an issue Feb 23, 2024 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Feb 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (42cfdc6) to head (ee63f3d).

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #65   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          123       123           
=========================================
  Hits           123       123           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SamuelBrand1
Copy link
Collaborator Author

I've just had a thought that I'm using a postfix of _dist in some places and _prior in others to indicate the prior/generative distribution of parameters... We should choose one and go with that (happy for this to become a new issue).

Copy link
Collaborator

@seabbs seabbs left a comment

Choose a reason for hiding this comment

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

LGTM

@seabbs seabbs enabled auto-merge February 23, 2024 14:08
@seabbs seabbs merged commit 39fd99a into main Feb 23, 2024
5 checks passed
@seabbs seabbs deleted the 63-moving-away-from-the-namedtuple-to-instead-use-argument-splitting branch February 23, 2024 14:16
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.

Moving away from the NamedTuple to instead use argument splitting.
3 participants