-
Notifications
You must be signed in to change notification settings - Fork 1
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
279 clarify auto adapt #280
Conversation
olivierilbert
commented
Mar 14, 2025
- Remove a1 and shift1 which were useless
- Apply shift0 when preparing the catalogue. So, shifts0 are always applied when APPLY_SYSSHIFT defined.
- a0 will be computed when AUTO_ADAPT YES, even if APPLY_SYSSHIFT exist. I tested that a0 are always lower than 0.01 when I applied previously computed offsets with APPLY_SYSSHIFT, which is a nice consistency check.
This should instead be done using APPLY_SYSHIFT to avoid conflict. A corresponding change will need to be made to rail_lephare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #280 +/- ##
==========================================
+ Coverage 58.95% 65.68% +6.73%
==========================================
Files 50 50
Lines 6478 6458 -20
Branches 946 944 -2
==========================================
+ Hits 3819 4242 +423
+ Misses 2659 2216 -443 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this 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 suggest merging the other minor changes to process in PR #278 after this is merged and then making a complementary PR in rail_lephare to propagate it after publishing a new version.
First getting the offsets in process
I think I have process at 100% now and have removed the offsets from the process parameters. |
I had also forgotton that I implemented this function which allows the user to use absolute paths to "load" a sed list: lephare/src/lephare/process.py Line 241 in f5b5b57
which may be relevant re the absolute path discussion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add the abs path code for a dedicated quick issue/branch, unless the unit tests for it are ready.
Otherwise, it is all good to go I think
The load_sed_list is already implemented and tested. I think we may have implemented partly with sharing between bpz and lephare in mind. |
@olivierilbert Are we good to go? |