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

Add more prior classes and add composite prior example #41

Closed
3 tasks done
kazewong opened this issue Nov 9, 2023 · 3 comments · Fixed by #43
Closed
3 tasks done

Add more prior classes and add composite prior example #41

kazewong opened this issue Nov 9, 2023 · 3 comments · Fixed by #43
Assignees

Comments

@kazewong
Copy link
Owner

kazewong commented Nov 9, 2023

Currently there is only uniform prior with a guard outside the domain available.

This create issue of broken waveform returning nan when the gradient is large in HMC, which shoot the test point outside the domain and raising nan.

Here is a list of priors that could be useful to have:

  • Unconstrained uniform. Sample from an unconstrained distribution that corresponds to a uniform distribution over [a,b]
  • Uniform on sphere. Sample from a sphere and transform them into Cartesian coordinate. Useful for spins.
  • Add example of composite prior. This should take a list of priors and return a prior
@kazewong kazewong self-assigned this Nov 9, 2023
@ThibeauWouters
Copy link
Collaborator

Peter and I were also interested in a DiracDelta prior, to fix e.g. the sky localisation to simplify the PE which might be useful to look into the sampling process itself and debug it. I haven't started on that, but that would also require a composite prior with some parameters having a uniform, and some having a DiracDelta prior. I'll ping u in case I start working on this at some point.

@ThibeauWouters
Copy link
Collaborator

Hi @kazewong , I was doing some PE runs on GW170817 and the luminosty distances are a bit off, because we are using the Uniform prior instead of a cosmological prior on the distance, as done in the TurboPE repo here. We were wondering, since you seemed to be working on prior classes, if it would be easy for you to add such a prior class to Jim? I am more than happy to help by implementing it myself in case you don't have time for it right now, but just wanted to check in case you are already working on something like this. Thanks a lot in advance!

@kazewong kazewong linked a pull request Nov 24, 2023 that will close this issue
@kazewong
Copy link
Owner Author

I think this can be handled. A slightly tricky bit is where do we want the dependency of cosmological parameters to live. I am fine with having astropy as a dependency to create the data for redshift/distance interpolation when initializing the class, but that would mean we cannot differentiate against the cosmological parameters. There exist some packages in jax that offers cosmology related calculation that can be diff/jit, but I am not comfortable integrating those into our dependency.

On the Dirac delta prior, I think in general we don't want that since that unnecessarily inflates the dimensionality of the problem. Further more, gradient-based samplers like HMC and MALA will probably encounter troubles with these types of prior. On top of this, I think this can be handled much more elegantly on the likelihood/model level. We can have some example for that later

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 a pull request may close this issue.

2 participants