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

Adding stresstest function to MechanisticInferer #199

Closed
wants to merge 1 commit into from

Conversation

SamuelBrand1
Copy link
Contributor

@SamuelBrand1 SamuelBrand1 commented Jul 17, 2024

This PR adds a stresstest method to the MechanisticInferer class which:

  • runs a trace on the numpyro model defined within a class instance to gather parameter names.
  • Generates a N long list of parameters with cauchy random values.
  • Passes these to potential_energy. NB: potential_energy expects parameters in the unconstrained domain, which is convenient here.
  • Returns all parameters which cause:
    • model run failure.
    • NaN return value.
    • Inf return value.

I've added a unit test too which only covers whether it runs and returns a list.

Caveats

I'm expecting this method to be inherited by any subtype of MechanisticInferer so that overloaded loglikelihood model methods should be covered by stresstest, but I'm not very strong at python so I might be misunderstanding inheritance here.

NB: For stresstest to work it requires kwargs to pass to the model via get_trace, which kwargs depends on the model (e.g. for the default model tf = a_number was sufficient).

Closes #197

Local pytest fail

I'm seeing this, but I can't seem to fix it on my local testing.

FAILED tests/test_integration.py::test_output_matches_previous_version - AssertionError: a change was detected in the 0 compartment. This can be for a couple of valid reasons:

@SamuelBrand1
Copy link
Contributor Author

Actually, this test is too harsh because it can vary the observables. I'll have a think about how to only hit the inferable params. One option might be to pass some pseudo-data and then filter out only the non-observed params. Open to suggestions here!

)
).get_trace(kwargs)
# Generate random parameter values with cauchy distribution
rand_vars = [
Copy link
Collaborator

Choose a reason for hiding this comment

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

Lets maybe look for a way to only randomly vary parameters we actually sample, rather than all numpyro parameters (which includes data)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we can do this by passing the data you want to pass into kwargs, sampling from the model prior and using that to create the stress test params.

@arik-shurygin
Copy link
Collaborator

hey @SamuelBrand1 I apologies for the extended period of nothing happening with this ticket. Do you still think it is necessary? I did not realize you had responded to my comment requesting changes

@SamuelBrand1
Copy link
Contributor Author

It would be a handy feature but its low priority since we identified the problem was with the ODE solver tol.

@SamuelBrand1
Copy link
Contributor Author

bayeux #244 Should cover this functionality if we use that.

@arik-shurygin arik-shurygin deleted the stresstest-function branch October 3, 2024 17:15
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.

Adding stress testing utility for model inference to help isolate cause of stochastic failure in inference
2 participants