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

Hotfix: Platescale binning correction #1721

Merged
merged 3 commits into from
Nov 8, 2023
Merged

Conversation

tbowers7
Copy link
Collaborator

@tbowers7 tbowers7 commented Nov 6, 2023

In pypeit.find_objects.MultiSlitFindObjects.get_platescale(), the platescale is scaled by the binning -- but the code accidentally was using the spectral binning rather than the spatial binning. This value is used to compute the BOXCAR RADIUS in the pypeit.core.findobj_skymask.objs_in_slit() function, which is then saved in the SpecObj object.

This PR corrects the binning scaling to use the SPATIAL binning.

modified:   pypeit/find_objects.py

In `pypeit.find_objects.MultiSlitFindObjects.get_platescale()`, the
platescale is scaled by the binning -- but the code accidentally was
using the *spectral* binning rather than the *spatial* binning.  This
value is used to compute the BOXCAR RADIUS in the
`pypeit.core.findobj_skymask.objs_in_slit()` function, which is then
saved in the `SpecObj` object.

This commit corrects the binning scaling to use the SPATIAL binning.

	modified:   pypeit/find_objects.py
@tbowers7 tbowers7 requested a review from kbwestfall November 6, 2023 21:09
Copy link
Collaborator

@kbwestfall kbwestfall left a comment

Choose a reason for hiding this comment

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

Looks good to me! I think we should run tests, unless we're confident that this isn't going to cause any object detection issues in the dev-suite.

@codecov-commenter
Copy link

codecov-commenter commented Nov 6, 2023

Codecov Report

Merging #1721 (bd6c142) into develop (ec1584d) will not change coverage.
The diff coverage is 100.00%.

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

@@           Coverage Diff            @@
##           develop    #1721   +/-   ##
========================================
  Coverage    41.03%   41.03%           
========================================
  Files          190      190           
  Lines        43724    43724           
========================================
  Hits         17942    17942           
  Misses       25782    25782           
Files Coverage Δ
pypeit/find_objects.py 46.09% <100.00%> (ø)

Copy link
Collaborator

@debora-pe debora-pe left a comment

Choose a reason for hiding this comment

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

Great catch @tbowers7!

@kbwestfall
Copy link
Collaborator

Hi @tbowers7 . I got one dev-suite failure:

Test Summary
--------------------------------------------------------
--- PYTEST PYPEIT UNIT TESTS PASSED  240 passed, 70 warnings in 556.04s (0:09:16) ---
--- PYTEST UNIT TESTS PASSED  133 passed, 171 warnings in 1027.10s (0:17:07) ---
--- PYTEST VET TESTS PASSED  58 passed, 69 warnings in 2245.07s (0:37:25) ---
--- PYPEIT DEVELOPMENT SUITE FAILED 1/233 TESTS  ---
Failed tests:
    ldt_deveny/DV7 pypeit
Skipped tests:
Testing Started at 2023-11-07T16:52:09.355981
Testing Completed at 2023-11-08T08:42:18.270379
Total Time: 15:50:08.914398

Here's the log entry:

-------------------------
Test Setup: ldt_deveny/DV7

-------------------------
Directories:
         Raw data: /tmp/PypeIt-development-suite/RAW_DATA/ldt_deveny/DV7
    PypeIt output: /tmp/REDUX_OUT/ldt_deveny/DV7
Files:
     .pypeit file: None
 Std .pypeit file: None
Tests:
----
ldt_deveny/DV7 pypeit Result: �[1;31m--- FAILED�[0m

Logfile:    /tmp/REDUX_OUT/ldt_deveny/DV7/ldt_deveny_dv7.test.log
Process Id: 1308
Start time: Wed Nov  8 06:04:26 2023
End time:   Wed Nov  8 06:04:41 2023
Duration:   0:00:14.484929
Mem Usage:  621867008
Command:    run_pypeit /tmp/REDUX_OUT/ldt_deveny/DV7/ldt_deveny_dv7.pypeit -o

Error Messages:

End of Log:
             Use the script `pypeit_install_linelist` to install
             your custom line list into the cache.  See instructions at
             https://pypeit.readthedocs.io/en/latest/wave_calib.html#line-lists

I'm not sure what caused the error. Can you look into this?

@tbowers7
Copy link
Collaborator Author

tbowers7 commented Nov 8, 2023

I'm not sure what caused the error. Can you look into this?

@kbwestfall : This is fixed in #1712.

@kbwestfall
Copy link
Collaborator

I'm not sure what caused the error. Can you look into this?

@kbwestfall : This is fixed in #1712.

Okay, great! Will merge soon.

@kbwestfall kbwestfall merged commit 058bbfb into develop Nov 8, 2023
23 checks passed
@kbwestfall kbwestfall deleted the platescale_hotfix branch November 8, 2023 18:29
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.

4 participants