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

Allowed q > 1 and added unit tests to check magnification #137

Merged
merged 8 commits into from
May 20, 2024

Conversation

rapoliveira
Copy link
Contributor

Removing two conditions of q <= 1 from modelparameters.py, one unit test was changed and two unit tests were added to ensure that the magnification is reproduced by the transformations q -> 1/q and alpha -> alpha +/- 180.

Parameters from example01 were used to test different mass ratios, except for the separation s, which was changed to reproduce a similar caustic crossing for binary lens.

See issue #84 for more details. Comments are welcome!

@rpoleski
Copy link
Owner

Few changes are needed:

  • if new_q < 0.: ----> <= 0.
  • correct error messages, so that they don't say q should be below 1.
  • New tests look quite long (many epochs there). Please try to make them faster, i.e., spend a few minutes finding which ~5 epochs probe potentially hard cases (e.g., close to caustics) so that running tests will be faster and we will save developers time in the long run :)

@rpoleski
Copy link
Owner

The last added test should be corrected. Qualitatively the difference between q of 0.99 and 1.01 should be similar to difference between 0.97 and 0.99. You may choose a trajectory that is passing close to the caustic but is not crossing it.

@rpoleski
Copy link
Owner

Please change version to 2.24.0. I'll merge after that.

@rpoleski rpoleski merged commit d6a1d35 into rpoleski:master May 20, 2024
1 check passed
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.

2 participants