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 Examples of single event runs #133

Conversation

xuyuon
Copy link
Contributor

@xuyuon xuyuon commented Aug 19, 2024

  • Fixed minor bugs in the test script which could yield nan in Adam optimization and ruin the run.
  • Added examples of single event runs: GW150914_D.py, GW150914_D_heterodyne.py, GW150914_Pv2.py

@xuyuon xuyuon requested review from kazewong and thomasckng August 19, 2024 05:15
@xuyuon xuyuon force-pushed the 98-moving-naming-tracking-into-jim-class-from-prior-class branch from fcb2651 to 645dea1 Compare August 21, 2024 07:41
src/jimgw/transforms.py Outdated Show resolved Hide resolved
@thomasckng thomasckng changed the base branch from 98-moving-naming-tracking-into-jim-class-from-prior-class to jim-dev September 3, 2024 00:38
@thomasckng thomasckng linked an issue Sep 3, 2024 that may be closed by this pull request
@thomasckng
Copy link
Collaborator

Btw, we may want to remove legacy examples either in this PR or another new PR.

@kazewong
Copy link
Owner

kazewong commented Sep 3, 2024

We can remove legacy example in this PR


M_c_min, M_c_max = 10.0, 80.0
q_min, q_max = 0.125, 1.0
m_1_prior = UniformPrior(Mc_q_to_m1_m2(M_c_min, q_max)[0], Mc_q_to_m1_m2(M_c_max, q_min)[0], parameter_names=["m_1"])
Copy link
Owner

Choose a reason for hiding this comment

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

This does not corresponds to Mc_max =80.0, please check for accuracy

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 this is related to the fact that when you sample from UniformPrior, it just randomly gives you a value within the range you have defined. Now, without any constraint, the range of m_1 and m_2 overlaps with each other, so m_2 can be bigger than m_1 when you try to sample on it. When m_2 is bigger than m_1, it gives strange value of M_c and q.

I think its better to build a special prior class for m_1 and m_2, in which we add the constraint m_1 > m_2 in sampling.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the value looks fine to me. I can test it with @xuyuon tomorrow.

BoundToUnbound(name_mapping = [["phase_c"], ["phase_c_unbounded"]] , original_lower_bound=0.0, original_upper_bound=2 * jnp.pi),
BoundToUnbound(name_mapping = [["iota"], ["iota_unbounded"]], original_lower_bound=0., original_upper_bound=jnp.pi),
BoundToUnbound(name_mapping = [["psi"], ["psi_unbounded"]], original_lower_bound=0.0, original_upper_bound=jnp.pi),
SkyFrameToDetectorFrameSkyPositionTransform(name_mapping = [["ra", "dec"], ["zenith", "azimuth"]], gps_time=gps, ifos=ifos),
Copy link
Owner

Choose a reason for hiding this comment

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

This does not work with the new predefined transform. Please change accordingly

@xuyuon
Copy link
Contributor Author

xuyuon commented Sep 11, 2024

I am going to close this PR, and separate it into two new PR.

This was referenced Sep 11, 2024
@xuyuon xuyuon closed this Sep 16, 2024
@xuyuon
Copy link
Contributor Author

xuyuon commented Sep 16, 2024

This PR is replaced by #157 and #154

@xuyuon xuyuon deleted the 98-moving-naming-tracking-into-jim-class-from-prior-class branch November 5, 2024 17:36
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