From a731f91b51bb3a9ae4dd1d151e2471b9dbd7ce76 Mon Sep 17 00:00:00 2001 From: "Raphael A. P. Oliveira" Date: Thu, 9 May 2024 12:20:47 +0200 Subject: [PATCH 1/8] Allowed q > 1 in modelparameters.py and changed one failing test --- source/MulensModel/modelparameters.py | 4 ++-- source/MulensModel/tests/test_ModelParameters.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source/MulensModel/modelparameters.py b/source/MulensModel/modelparameters.py index bdc67673..b6c52be6 100644 --- a/source/MulensModel/modelparameters.py +++ b/source/MulensModel/modelparameters.py @@ -878,7 +878,7 @@ def _check_valid_parameter_values(self, parameters): for name in ['q']: if name in parameters.keys(): - if parameters[name] <= 0. or parameters[name] >= 1.: + if parameters[name] <= 0.: msg = "Parameter {:} has to be in (0, 1) range, not {:}" raise ValueError(msg.format(name, parameters[name])) @@ -1234,7 +1234,7 @@ def q(self): @q.setter def q(self, new_q): - if new_q < 0. or new_q > 1.: + if new_q < 0.: raise ValueError('mass ratio q has to be between 0 and 1') self.parameters['q'] = new_q self._update_sources('q', new_q) diff --git a/source/MulensModel/tests/test_ModelParameters.py b/source/MulensModel/tests/test_ModelParameters.py index 670b19a5..a0c65dbe 100644 --- a/source/MulensModel/tests/test_ModelParameters.py +++ b/source/MulensModel/tests/test_ModelParameters.py @@ -30,7 +30,7 @@ def test_wrong_type_of_parameters(self): 'shear_G': 0.1, 'alpha': 123.}) with self.assertRaises(ValueError): mm.ModelParameters({'t_0': 123., 'u_0': 1, 't_E': 10., 's': 1.2, - 'alpha': 34.56, 'q': 1.5}) + 'alpha': 34.56, 'q': -0.5}) def test_init_for_2_sources(self): """ From 5161aa4357242a6a36685dca723f41aa7115d087 Mon Sep 17 00:00:00 2001 From: "Raphael A. P. Oliveira" Date: Thu, 9 May 2024 13:10:23 +0200 Subject: [PATCH 2/8] Added unit test to check if q > 1 is consistent --- .../MulensModel/tests/test_ModelParameters.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/source/MulensModel/tests/test_ModelParameters.py b/source/MulensModel/tests/test_ModelParameters.py index a0c65dbe..9ab8069d 100644 --- a/source/MulensModel/tests/test_ModelParameters.py +++ b/source/MulensModel/tests/test_ModelParameters.py @@ -150,6 +150,38 @@ def test_positive_t_E(): assert params.t_E == params.t_eff / abs(params.u_0) +def test_q_gt_1_is_good(): + """ + Check if the magnification is reproduced by transforming q -> 1/q and + alpha -> alpha +/- 180, including for q > 1. See issue #84. + """ + t_0 = 3583. + u_0 = 0.3 + t_E = 12. + s = 1.65 + q = 0.25 + alpha = 339.0 + rho = 0.001 + + planet = mm.Model({'t_0': t_0, 'u_0': u_0, 't_E': t_E, 's': s, 'q': q, + 'alpha': alpha, 'rho': rho}) + planet_2 = mm.Model({'t_0': t_0, 'u_0': u_0, 't_E': t_E, 's': s, 'q': 1/q, + 'alpha': alpha+180., 'rho': rho}) + planet_3 = mm.Model({'t_0': t_0, 'u_0': u_0, 't_E': t_E, 's': s, 'q': 1/q, + 'alpha': alpha-180., 'rho': rho}) + list_of_methods = [3588., 'VBBL', 3594., 'hexadecapole', 3598.0] + planet.set_magnification_methods(list_of_methods) + planet_2.set_magnification_methods(list_of_methods) + planet_3.set_magnification_methods(list_of_methods) + t_range = np.arange(3580., 3600., 0.1) + magnifications_1 = planet.get_magnification(time=t_range) + magnifications_2 = planet_2.get_magnification(time=t_range) + magnifications_3 = planet_3.get_magnification(time=t_range) + + assert max(magnifications_1 - magnifications_2) < 1e-10 + assert max(magnifications_1 - magnifications_3) < 1e-10 + + def test_rho_t_e_t_star(): """check if conversions between rho, t_E, and t_star work ok""" t_0 = 2450000. From 31b9d8d3c24f93798cc27af2d03728a7047de03b Mon Sep 17 00:00:00 2001 From: "Raphael A. P. Oliveira" Date: Thu, 9 May 2024 13:59:17 +0200 Subject: [PATCH 3/8] Similar unit test to check smooth transition for q=0.99 --- .../MulensModel/tests/test_ModelParameters.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/source/MulensModel/tests/test_ModelParameters.py b/source/MulensModel/tests/test_ModelParameters.py index 9ab8069d..3115d5b2 100644 --- a/source/MulensModel/tests/test_ModelParameters.py +++ b/source/MulensModel/tests/test_ModelParameters.py @@ -182,6 +182,31 @@ def test_q_gt_1_is_good(): assert max(magnifications_1 - magnifications_3) < 1e-10 +def test_q_gt_1_is_smooth(): + """ + Check that there is a smooth transition from q = 0.99 and q = 1.01. + """ + t_0 = 3583. + u_0 = 0.3 + t_E = 12. + s = 2.18 + q = 0.99 # 0.999 also works fine + alpha = 339.0 + rho = 0.001 + planet = mm.Model({'t_0': t_0, 'u_0': u_0, 't_E': t_E, 's': s, 'q': q, + 'alpha': alpha, 'rho': rho}) + planet_2 = mm.Model({'t_0': t_0, 'u_0': u_0, 't_E': t_E, 's': s, 'q': 1/q, + 'alpha': alpha+180., 'rho': rho}) + + planet.set_magnification_methods([3590., 'VBBL', 3595.]) + planet_2.set_magnification_methods([3590., 'VBBL', 3595.]) + t_range = np.arange(3580., 3600., 0.1) + magnifications_1 = planet.get_magnification(time=t_range) + magnifications_2 = planet_2.get_magnification(time=t_range) + + assert max(magnifications_1 - magnifications_2) < 1e-10 + + def test_rho_t_e_t_star(): """check if conversions between rho, t_E, and t_star work ok""" t_0 = 2450000. From 9160298a8171cc7cef14e0a0c96a940d728a1eb9 Mon Sep 17 00:00:00 2001 From: "Raphael A. P. Oliveira" Date: Thu, 9 May 2024 14:08:27 +0200 Subject: [PATCH 4/8] Added imports to __all__ in __init__.py, see PR #130 --- source/MulensModel/__init__.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/source/MulensModel/__init__.py b/source/MulensModel/__init__.py index 738d23f6..930c0d45 100644 --- a/source/MulensModel/__init__.py +++ b/source/MulensModel/__init__.py @@ -26,7 +26,15 @@ from .version import __version__ -__all__ = ['mulensobjects', 'MODULE_PATH', 'DATA_PATH', 'BinaryLens'] +__all__ = ['mulensobjects', 'MODULE_PATH', 'DATA_PATH', 'BinaryLens', + 'BinaryLensWithShear', 'Caustics', 'CausticsPointWithShear', + 'CausticsWithShear', 'Coordinates', 'Event', 'FitData', 'Horizons', + 'LimbDarkeningCoeffs', 'MagnificationCurve', 'Model', + 'ModelParameters', 'which_parameters', 'MulensData', 'Lens', + 'Source', 'MulensSystem', 'orbits', 'PointLens', + 'get_pspl_magnification', 'PointLensWithShear', + 'PointLensFiniteSource', 'SatelliteSkyCoord', 'Trajectory', + 'UniformCausticSampling', 'MAG_ZEROPOINT', 'Utils', '__version__'] MODULE_PATH = path.abspath(__file__) for i in range(3): From b16216ad2d538e258135e0fe59435376445c1ca9 Mon Sep 17 00:00:00 2001 From: "Raphael A. P. Oliveira" Date: Fri, 10 May 2024 15:06:10 +0200 Subject: [PATCH 5/8] Radek suggestions for q > 1, shorter tests, issue #84 --- source/MulensModel/modelparameters.py | 6 +++--- source/MulensModel/tests/test_ModelParameters.py | 4 ++-- source/MulensModel/version.py | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/source/MulensModel/modelparameters.py b/source/MulensModel/modelparameters.py index b6c52be6..9c42e868 100644 --- a/source/MulensModel/modelparameters.py +++ b/source/MulensModel/modelparameters.py @@ -879,7 +879,7 @@ def _check_valid_parameter_values(self, parameters): for name in ['q']: if name in parameters.keys(): if parameters[name] <= 0.: - msg = "Parameter {:} has to be in (0, 1) range, not {:}" + msg = "Parameter {:} has to be larger than 0, not {:}" raise ValueError(msg.format(name, parameters[name])) for name in ['xi_eccentricity']: @@ -1234,8 +1234,8 @@ def q(self): @q.setter def q(self, new_q): - if new_q < 0.: - raise ValueError('mass ratio q has to be between 0 and 1') + if new_q <= 0.: + raise ValueError('mass ratio q has to be larger than 0') self.parameters['q'] = new_q self._update_sources('q', new_q) diff --git a/source/MulensModel/tests/test_ModelParameters.py b/source/MulensModel/tests/test_ModelParameters.py index 3115d5b2..1ebe6adf 100644 --- a/source/MulensModel/tests/test_ModelParameters.py +++ b/source/MulensModel/tests/test_ModelParameters.py @@ -173,7 +173,7 @@ def test_q_gt_1_is_good(): planet.set_magnification_methods(list_of_methods) planet_2.set_magnification_methods(list_of_methods) planet_3.set_magnification_methods(list_of_methods) - t_range = np.arange(3580., 3600., 0.1) + t_range = [3580, 3589, 3590, 3592, 3593, 3595] magnifications_1 = planet.get_magnification(time=t_range) magnifications_2 = planet_2.get_magnification(time=t_range) magnifications_3 = planet_3.get_magnification(time=t_range) @@ -200,7 +200,7 @@ def test_q_gt_1_is_smooth(): planet.set_magnification_methods([3590., 'VBBL', 3595.]) planet_2.set_magnification_methods([3590., 'VBBL', 3595.]) - t_range = np.arange(3580., 3600., 0.1) + t_range = [3571, 3590.5, 3591, 3592, 3593.4, 3594.5] magnifications_1 = planet.get_magnification(time=t_range) magnifications_2 = planet_2.get_magnification(time=t_range) diff --git a/source/MulensModel/version.py b/source/MulensModel/version.py index db969b3a..def8f78c 100644 --- a/source/MulensModel/version.py +++ b/source/MulensModel/version.py @@ -1 +1 @@ -__version__ = "2.23.0" +__version__ = "2.23.1" From 31e994fbeb51feba688fb89c5f8ec795cbd45694 Mon Sep 17 00:00:00 2001 From: radek_poleski Date: Sun, 19 May 2024 11:02:18 +0200 Subject: [PATCH 6/8] contributing - inspired by mjmroz --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9e9cf961..4aa4d594 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,7 +13,7 @@ We're also open to suggestions, new ideas, and requests. If you have them, then ### Do you have code to contribute? -Having a code that you want to share is great! We follow coding conventions (e.g., [PEP8](https://github.com/rpoleski/MulensModel/issues/new), docstrings for [sphinx](https://www.sphinx-doc.org/en/master/), [camel case](https://en.wikipedia.org/wiki/Snake_case) names of classes, [snake case](https://en.wikipedia.org/wiki/Snake_case) names of attributes and methods, etc.) but you don't have to follow all of them right away. Just open a new [pull request](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests) and we will have some discussion on changes that need to be done before merging. Please note that we expect that you will provide a unit test for every new public function that is not plotting. If the function is plotting, then we expect a new example. Sometimes there is significant discussion befor the pull request is accepted (see some [closed ones](https://github.com/rpoleski/MulensModel/pulls?q=is%3Apr+is%3Aclosed) for examples). +Having a code that you want to share is great! We follow coding conventions (e.g., [PEP8](https://github.com/rpoleski/MulensModel/issues/new), [sphinx](https://www.sphinx-doc.org/en/master/)-compatible docstrings for public functions, [camel case](https://en.wikipedia.org/wiki/Snake_case) names of classes, [snake case](https://en.wikipedia.org/wiki/Snake_case) names of attributes and methods, etc.) but you don't have to follow all of them right away. Just open a new [pull request](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests) and we will have some discussion on changes that need to be done before merging. Please note that we expect that you will provide a unit test for every new public function that is not plotting. If the function is plotting, then we expect a new example. Sometimes there is significant discussion befor the pull request is accepted (see some [closed ones](https://github.com/rpoleski/MulensModel/pulls?q=is%3Apr+is%3Aclosed) for examples). An important aspect of adding new code is writing tests that can be run in automated way. The tests are in `source/MulensModel/tests/` directory and you can run them by invoking `pytest` command (it requires pytest package to be installed). If you don't know how to write tests for your code, don't worry - we will help you. We expect that new code is tested in nearly 100% when it's merged. From a071bd0ddb3e68f608cbdf5a455287516397b912 Mon Sep 17 00:00:00 2001 From: "Raphael A. P. Oliveira" Date: Mon, 20 May 2024 16:53:50 +0200 Subject: [PATCH 7/8] Second test corrected to 0.99-1.01 and 0.97-1.01, issue #84 --- .../MulensModel/tests/test_ModelParameters.py | 41 +++++++++++-------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/source/MulensModel/tests/test_ModelParameters.py b/source/MulensModel/tests/test_ModelParameters.py index 1ebe6adf..89036a2a 100644 --- a/source/MulensModel/tests/test_ModelParameters.py +++ b/source/MulensModel/tests/test_ModelParameters.py @@ -173,10 +173,10 @@ def test_q_gt_1_is_good(): planet.set_magnification_methods(list_of_methods) planet_2.set_magnification_methods(list_of_methods) planet_3.set_magnification_methods(list_of_methods) - t_range = [3580, 3589, 3590, 3592, 3593, 3595] - magnifications_1 = planet.get_magnification(time=t_range) - magnifications_2 = planet_2.get_magnification(time=t_range) - magnifications_3 = planet_3.get_magnification(time=t_range) + t_checks = [3580, 3589, 3590, 3592, 3593, 3595] + magnifications_1 = planet.get_magnification(time=t_checks) + magnifications_2 = planet_2.get_magnification(time=t_checks) + magnifications_3 = planet_3.get_magnification(time=t_checks) assert max(magnifications_1 - magnifications_2) < 1e-10 assert max(magnifications_1 - magnifications_3) < 1e-10 @@ -184,27 +184,34 @@ def test_q_gt_1_is_good(): def test_q_gt_1_is_smooth(): """ - Check that there is a smooth transition from q = 0.99 and q = 1.01. + Check that there is a smooth transition between q = 0.97, 0.99 and 1.01. + In this case, a trajectory with caustic approaching is adopted. """ t_0 = 3583. u_0 = 0.3 t_E = 12. s = 2.18 - q = 0.99 # 0.999 also works fine - alpha = 339.0 + q = 0.99 + alpha = 310. rho = 0.001 planet = mm.Model({'t_0': t_0, 'u_0': u_0, 't_E': t_E, 's': s, 'q': q, 'alpha': alpha, 'rho': rho}) - planet_2 = mm.Model({'t_0': t_0, 'u_0': u_0, 't_E': t_E, 's': s, 'q': 1/q, - 'alpha': alpha+180., 'rho': rho}) - - planet.set_magnification_methods([3590., 'VBBL', 3595.]) - planet_2.set_magnification_methods([3590., 'VBBL', 3595.]) - t_range = [3571, 3590.5, 3591, 3592, 3593.4, 3594.5] - magnifications_1 = planet.get_magnification(time=t_range) - magnifications_2 = planet_2.get_magnification(time=t_range) - - assert max(magnifications_1 - magnifications_2) < 1e-10 + q_min = mm.Model({'t_0': t_0, 'u_0': u_0, 't_E': t_E, 's': s, 'q': q-0.02, + 'alpha': alpha, 'rho': rho}) + q_max = mm.Model({'t_0': t_0, 'u_0': u_0, 't_E': t_E, 's': s, 'q': q+0.02, + 'alpha': alpha, 'rho': rho}) + + planet.set_magnification_methods([3580., 'VBBL', 3595.]) + q_min.set_magnification_methods([3580., 'VBBL', 3595.]) + q_max.set_magnification_methods([3580., 'VBBL', 3595.]) + t_checks = [3571, 3583, 3585.5, 3586, 3586.5, 3592.5] + magnification = planet.get_magnification(time=t_checks) + diff_min = magnification - q_min.get_magnification(time=t_checks) + diff_max = magnification - q_max.get_magnification(time=t_checks) + limits = np.array([0.01, 0.01, 0.01, 0.56, 0.01, 0.03]) + + assert np.all(abs(diff_min) < limits) + assert np.all(abs(diff_max) < limits) def test_rho_t_e_t_star(): From af0cfae345e7a7ce473ed04b3fb32290d421cd3f Mon Sep 17 00:00:00 2001 From: "Raphael A. P. Oliveira" Date: Mon, 20 May 2024 18:07:54 +0200 Subject: [PATCH 8/8] Corrected version number, issue q > 1 solved --- source/MulensModel/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/MulensModel/version.py b/source/MulensModel/version.py index def8f78c..000408e0 100644 --- a/source/MulensModel/version.py +++ b/source/MulensModel/version.py @@ -1 +1 @@ -__version__ = "2.23.1" +__version__ = "2.24.0"