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

Folding IHR into config? #273

Open
kokbent opened this issue Oct 11, 2024 · 3 comments
Open

Folding IHR into config? #273

kokbent opened this issue Oct 11, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@kokbent
Copy link
Collaborator

kokbent commented Oct 11, 2024

Somewhat related to #161

IHR priors and stuff are currently specified in the likelihood function directly instead of in the config json. There is room to move them into the config, although I can see some challenges there since IHR parameters can increase/decrease depending on how we feel, and the IHR models can change so it needs to be a bit flexible...

@arik-shurygin
Copy link
Collaborator

what sort of flexibility would you like to allow for? References to other parameters may be difficult due to the structure of JSON.

four independent IHR values are easy, sampling multiplier values that are independent but multiply with the other IHR's is also easy. But if we wanted something more complex than that we may need to get creative.

@SamuelBrand1
Copy link
Contributor

Being able to specify the priors to the model is a fairly simple way would be a really good feature.

On way of doing this would be having a restricted set of strings like beta, gamma, normal in the configuration files along with the associated hyperparameters (e.g. a, b for Beta, mean std for Normal), then you can select the model prior choice with a string match.

@arik-shurygin
Copy link
Collaborator

Being able to specify the priors to the model is a fairly simple way would be a really good feature.

On way of doing this would be having a restricted set of strings like beta, gamma, normal in the configuration files along with the associated hyperparameters (e.g. a, b for Beta, mean std for Normal), then you can select the model prior choice with a string match.

I dont think that is what Ben is asking for. We have had the ability to specify prior distributions since #81 and even before. The issue is these prior distributions are independent of one another. You can't currently declare related variables from the config e.g.:

ihr_1 = numpyro.distributions.Normal(0, 1)
ihr_2 = numpyro.deterministic(ihr_1 / 2)

you could however do something like this using config logic along with minor code changes

ihr_1= numpyro.distributions.Normal(0, 1)
ihr_2_multiplier = numpyro.distributions.Normal(0, 0.5)
# within the set_downstream_parameters() function call
ihr2 = numpyro.deterministic(ihr_2_multiplier * ihr_1)

this is all pseudocode meant for demonstration purposes.

In short: JSON does not allow natively for references to other parameters, making doing something like this quite hacky and not encouraged. This was an issue identified very early on in development but was accepted as one of the tradeoffs with using JSON config files.

@arik-shurygin arik-shurygin added the enhancement New feature or request label Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants