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

ENH: use numpyro implementation of dismod at to derive a consistent remission rate #1

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

aflaxman
Copy link
Member

@aflaxman aflaxman commented Oct 9, 2024

Title: Use a custom implementation of the DisMod-AT approach to get remission, since it is not available from get_draws

Description

  • Category: feature
  • JIRA issue: none
  • Research reference: none

Changes and notes

This refactors a bunch of statistical computation code from my notebooks into a .py file and eventually will use it.

Verification and Testing

I'm not sure how to store multiple keys in the artifact which I think I should do, since the code here will find different draws from incidence, prevalence, etc that are consistent with the draws of remission it generates.

see 2024_10_30b_test_dismod_for_consistent_artifact_data.ipynb for example
see 2024_10_30b_test_dismod_for_consistent_artifact_data.ipynb for details
@aflaxman aflaxman changed the title WIP: use numpyro implementation of dismod at to derive a consistent remission rate ENH: use numpyro implementation of dismod at to derive a consistent remission rate Nov 7, 2024
def dismod_f(t, y, args):
S, C = y
i, r, f, m = args
return (-m * S - i * S + r * C, -m * C + i * S - r * C - f * C)
Copy link
Member Author

Choose a reason for hiding this comment

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

I object to the approach isort/black has taken to formatting this

Copy link

Choose a reason for hiding this comment

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

This is probably controversial because I know people are used to writing differential equations in a certain way, but if this is going to be code maybe you should make each item in this tuple a variable, and give everything here meaningful names instead of single letters?

if rate_name in art.keys:
art.replace(rate_name, df_out)
else:
art.write(rate_name, df_out)
Copy link

Choose a reason for hiding this comment

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

Is there a reason you put this in the builder.py and not loader.py?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a reason you put this in the builder.py and not loader.py?

based on this slack discussion, it seemed most convenient (since it is available in make_artifacts). How should I think about the difference between builder and loader?

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.

3 participants