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

Merge emcee_batch Into dev #404

Merged
merged 68 commits into from
Dec 4, 2024
Merged

Merge emcee_batch Into dev #404

merged 68 commits into from
Dec 4, 2024

Conversation

TimothyWillard
Copy link
Contributor

@TimothyWillard TimothyWillard commented Nov 21, 2024

Describe your changes.

This pull request makes the changes required to shape the emcee_batch branch into an appropriate format for dev.

Does this pull request make any user interface changes? If so please describe.

The user interface changes are:

  • The 'compartments' and 'seir' (and specifically a 'transitions' subsection) configuration sections are now required to create an instance of GempyorInference. Likely not a major change for users as any users doing inference are usually specifying compartments and transitions, but is a change for unit testing.

Those are reflected in updates to the documentation in the "Inference with EMCEE" GitBook documentation page.

What does your pull request address? Tag relevant issues.

This pull request addresses GH-403.

jcblemai and others added 30 commits September 16, 2024 00:09
@TimothyWillard
Copy link
Contributor Author

Didn't run automatically since I had this marked as draft, but manually ran them and GH actions do pass:

  1. gempyor: https://github.com/HopkinsIDD/flepiMoP/actions/runs/11978079961
  2. inference: https://github.com/HopkinsIDD/flepiMoP/actions/runs/11978250060

With commit 05b760b.

Copy link
Collaborator

@anjalika-nande anjalika-nande left a comment

Choose a reason for hiding this comment

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

I mainly reviewed the documentation part (since we're doing emcee operational checks once it's merged in dev) and I like the changes to the documentation -- the differences between this and classical inference are more clear now.

Copy link
Contributor

@pearsonca pearsonca left a comment

Choose a reason for hiding this comment

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

Some minor items to address.

.gitignore Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

what's going on with this file? not whole sale deleted, but definitely substantially reduced.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file was heavily modified in ce59add. It's written by the examples/simple_usa_statelevel/inference_benchmark.ipynb notebook. Although, I'm having difficultly running it locally since it relies on files outside of the flepiMoP repository (probably on @jcblemai laptop) and dependencies not installed by flepiMoP. I've created an issue to follow up on this: #410.

examples/simple_usa_statelevel/simple_usa_statelevel.yml Outdated Show resolved Hide resolved
flepimop/gempyor_pkg/src/gempyor/NPI/base.py Outdated Show resolved Hide resolved
Original issue was addressed by adding the needed file with `--force`
and `model_output` related gitignores for the inference tests can be
removed.
Utilize confuse more by relying on it's `as_str_seq` getter for pulling
a sequence of strings in a yaml file as a list of strings in python per
an @pearsonca suggestion.
Added a unit test to test run the `flepimop modifiers config-plot`
command and check that it produces the expected output files of plots.
Move the call to `get_static_arguments` under the 'emcee' inference
branch. This avoids calling `outcomes.read_parameters_from_config`
twice. That function has a side effect of editing the outcome modifiers
config. See
#373 (comment)
for details.
@TimothyWillard TimothyWillard mentioned this pull request Dec 4, 2024
Init `static_sim_arguments` attribute with `None` and then manually
extract the unique strings from the compartments transition array in the
`flepimop modifiers config-plot` CLI tool when interacting with
non-EMCEE inference configs.
@TimothyWillard
Copy link
Contributor Author

@TimothyWillard TimothyWillard merged commit 5f5f6d6 into dev Dec 4, 2024
3 checks passed
@TimothyWillard TimothyWillard deleted the feature/403/emcee_batch branch December 4, 2024 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
batch Relating to batch processing. gempyor Concerns the Python core. high priority High priority. inference Concerns the parameter inference framework.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature request]: Merge emcee_batch Into dev
6 participants