rpoleski / MulensModel

Microlensing Modelling package
https://rpoleski.github.io/MulensModel/
Other
56 stars 15 forks source link

Issue #94: proposed 1L2S get_trajectory bug fixes #98

Closed jenniferyee closed 10 months ago

jenniferyee commented 1 year ago

Proposed bug fixes for Model.get_trajectory() and FitData.get_dataset_trajectory(). Also updated Ex 17 to include plotting of 1L2S trajectory figure and added unittests for Model.get_trajectory() and FitData.get_dataset_trajectory(). These unit tests currently fail. We need to update the reference files to include trajectory X, Y, first.

jenniferyee commented 11 months ago

Specifically, what is needed:

SAMPLE_FILE_03_REF = join(dir_2, 'ob140939_Spitzer_ref_v1.dat')

Needs columns ADDED for Spitzer source trajectory X and Y positions as in

ref_Spitzer = np.loadtxt(SAMPLE_FILE_03_REF, unpack=True)
ratio_x = trajectory.x / ref_Spitzer[6]
ratio_y = trajectory.y / ref_Spitzer[7]
rpoleski commented 11 months ago

@jenniferyee I would just test against values returned by current code - are you ok with that? It'll take me relatively long to get them from independent code like sfit.

codecov-commenter commented 10 months ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (537e33c) 83.45% compared to head (bf40fcb) 83.63%. Report is 12 commits behind head on master.

Files Patch % Lines
source/MulensModel/model.py 90.00% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #98 +/- ## ========================================== + Coverage 83.45% 83.63% +0.18% ========================================== Files 52 52 Lines 8176 8269 +93 ========================================== + Hits 6823 6916 +93 Misses 1353 1353 ``` | [Flag](https://app.codecov.io/gh/rpoleski/MulensModel/pull/98/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+Poleski) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/rpoleski/MulensModel/pull/98/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+Poleski) | `83.63% <99.01%> (+0.18%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Radek+Poleski#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

rpoleski commented 10 months ago

Without running the code, I see that Model.get_trajectory() should have satellite_skycoord keyword describe. It's a new aspect of API, so we should change the version to 2.19.

jenniferyee commented 10 months ago

HOORAY!

Dr. Jennifer C. Yee (she/her)

Office: P-341 Center for Astrophysics | Harvard & Smithsonian 60 Garden St, MS-15 Cambridge, MA 02138

On Thu, Dec 7, 2023 at 9:53 AM Radek Poleski @.***> wrote:

Merged #98 https://github.com/rpoleski/MulensModel/pull/98 into master.

— Reply to this email directly, view it on GitHub https://github.com/rpoleski/MulensModel/pull/98#event-11183040727, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEK7UCJATSDXCBREFAPUPRLYIHJ7ZAVCNFSM6AAAAAA5BZBCBGVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGE4DGMBUGA3TENY . You are receiving this because you were mentioned.Message ID: @.***>