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

Retrieve energies of triggered events from dl1 parameters #188

Merged
merged 7 commits into from
Jul 20, 2023

Conversation

mikael10j
Copy link
Collaborator

@mikael10j mikael10j commented Apr 11, 2022

  • Remove useless code
  • Get triggered energies from dl1 params if possible

These energies allow for a computation of the max effective area.

@mikael10j mikael10j requested a review from vuillaut April 11, 2022 13:32
@codecov
Copy link

codecov bot commented Apr 11, 2022

Codecov Report

Patch coverage: 29.25% and project coverage change: +0.03 🎉

Comparison is base (eafbe37) 63.15% compared to head (126f7b7) 63.19%.

❗ Current head 126f7b7 differs from pull request most recent head 31aa52c. Consider uploading reports for the commit 31aa52c to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #188      +/-   ##
==========================================
+ Coverage   63.15%   63.19%   +0.03%     
==========================================
  Files          21       21              
  Lines        2717     2733      +16     
==========================================
+ Hits         1716     1727      +11     
- Misses       1001     1006       +5     
Impacted Files Coverage Δ
ctaplot/gammaboard/reader.py 0.00% <0.00%> (ø)
ctaplot/plots/style.py 100.00% <ø> (ø)
ctaplot/gammaboard/gammaboard.py 11.81% <2.97%> (-0.10%) ⬇️
ctaplot/ana/ana.py 88.20% <85.71%> (-0.03%) ⬇️
ctaplot/__init__.py 100.00% <100.00%> (ø)
ctaplot/ana/tests/test_ana.py 100.00% <100.00%> (ø)
ctaplot/plots/plots.py 78.46% <100.00%> (ø)
ctaplot/plots/tests/test_plots.py 98.73% <100.00%> (+0.07%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

ctaplot/gammaboard/gammaboard.py Outdated Show resolved Hide resolved
@kosack
Copy link

kosack commented Apr 13, 2022

Just a note: you might want to migrate to using ctapipe.io.TableLoader to read the data - it's designed for that purpose and would replace most of the code here. Or at the very least, use ctapipe.io.read_table instead of doing it all manually.

@vuillaut
Copy link
Member

Just a note: you might want to migrate to using ctapipe.io.TableLoader to read the data - it's designed for that purpose and would replace most of the code here. Or at the very least, use ctapipe.io.read_table instead of doing it all manually.

Is TableLoader tight to ctapipe data model?
We are mostly using files following lstchain data model at the moment...

@kosack
Copy link

kosack commented Apr 14, 2022

Is TableLoader tight to ctapipe data model?
We are mostly using files following lstchain data model at the moment...

Yes, it's basically kept up to date with changes to the ctapipe model (though it's only loosly tied - it doesn't care about the contents of the tables, but the table names do assume ctapipe conventions. The best would be just to swap any write function in lstchain with ctapipe.io.DataWriter, so you get the ctapipe data model.

Alternately, you should a least use ctapipe.io.read_table, which is lower-level, and doesn't assume the ctapipe data model (but does exactly the inverse transform of what ctapipe.io.TableWriter does when writing). I assume LSTChain uses TableWriter? Or is it's IO completely custom?

Comment on lines 86 to 91
for obs_id in np.unique(dl1_params['obs_id']):
mask = dl1_params['obs_id'] == obs_id
dl1_filtered = dl1_params[mask]
_, indices = np.unique(dl1_filtered['event_id'],
return_index=True)
trig_energies.append(dl1_filtered['mc_energy'][indices])
Copy link
Member

Choose a reason for hiding this comment

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

similar to our discussion, this can be simplified using axis

mask = np.unique(dl1_params[['obs_id', 'event_id']], axis=0, return_index=True)
trig_energies = dl1_params['mc_energy'][mask]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@vuillaut vuillaut merged commit 3d57501 into master Jul 20, 2023
4 of 5 checks passed
@vuillaut vuillaut deleted the trig_events branch July 20, 2023 10:47
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.

3 participants