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

Fix issue with FoV offset binnning when using energy dependant theta cut and diffuse MC #1328

Merged
merged 5 commits into from
Dec 23, 2024

Conversation

mdebony
Copy link
Collaborator

@mdebony mdebony commented Dec 19, 2024

This PR fix the issue #1322 . The bloc of code managing the FoV binning as been moved too.

The PR also add a warning message if the FoV binning is wider than the simulated FoV, and restrict the gammas used for the computation of energy dependant cuts (gh and theta) to the ones within the FoV range considered for the IRF.

Copy link

codecov bot commented Dec 19, 2024

Codecov Report

Attention: Patch coverage is 55.55556% with 4 lines in your changes missing coverage. Please review.

Project coverage is 73.51%. Comparing base (9aaa78b) to head (28ce7b4).
Report is 15 commits behind head on main.

Files with missing lines Patch % Lines
lstchain/tools/lstchain_create_irf_files.py 55.55% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1328      +/-   ##
==========================================
- Coverage   73.52%   73.51%   -0.01%     
==========================================
  Files         134      134              
  Lines       14215    14215              
==========================================
- Hits        10451    10450       -1     
- Misses       3764     3765       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mdebony
Copy link
Collaborator Author

mdebony commented Dec 19, 2024

@moralejo This is the new PR. Now test are working, but the codecov check are failling. I'm not sure what I need to change.

fov_offset_bins = [mean_fov_offset - 0.1, mean_fov_offset + 0.1] * u.deg
else:
fov_offset_bins = self.data_bin.fov_offset_bins()
self.log.info("Multiple offset for diffuse gamma MC")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps you can print in the log the binning (might be somewhere else, but better be explicit on what it is doing)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added in the log line the FoV binning used.

@moralejo
Copy link
Collaborator

@moralejo This is the new PR. Now test are working, but the codecov check are failling. I'm not sure what I need to change.

codecov is not strictly required. I think you would have to cover a larger part of the code with CI tests

@moralejo moralejo merged commit 289b032 into main Dec 23, 2024
6 of 8 checks passed
@moralejo moralejo deleted the diffuseMC_edepThethaCut branch December 23, 2024 10:19
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.

2 participants