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

Dealing with configs in a better way #983

Merged
merged 11 commits into from
May 26, 2022
Merged

Dealing with configs in a better way #983

merged 11 commits into from
May 26, 2022

Conversation

vuillaut
Copy link
Member

I think this should solve #924 (implementing solution B discussed there) #208 and improve the way we deal with configs.

  • MC now gets its own config file so we stop forgetting about apply_integration_correction
  • Users can still work with a limited config file (e.g. containing only an updated tailcut config different from the standard one)
  • Users can then use the --update-with to dump a full configuration from that limited config

@SeiyaNozaki could you have a look at the update srcdep config please?

Fixes #924
Fixes #208

@vuillaut vuillaut requested a review from SeiyaNozaki May 20, 2022 16:55
@vuillaut
Copy link
Member Author

Note I have also taken the liberty to update min_samples_leaf = 10 since this is what is used for the new MC prod by default.

@codecov
Copy link

codecov bot commented May 20, 2022

Codecov Report

Merging #983 (4371d89) into master (18a970c) will increase coverage by 0.03%.
The diff coverage is 94.28%.

@@            Coverage Diff             @@
##           master     #983      +/-   ##
==========================================
+ Coverage   85.66%   85.69%   +0.03%     
==========================================
  Files          78       78              
  Lines        6570     6598      +28     
==========================================
+ Hits         5628     5654      +26     
- Misses        942      944       +2     
Impacted Files Coverage Δ
lstchain/io/config.py 94.87% <88.88%> (-5.13%) ⬇️
lstchain/io/tests/test_config.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 18a970c...4371d89. Read the comment docs.

Copy link
Collaborator

@SeiyaNozaki SeiyaNozaki left a comment

Choose a reason for hiding this comment

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

Thank you @vuillaut !
It looks fine from my side.

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.

dealing with different "standard" configs for MC and telescope data Single config file ?
3 participants