-
Notifications
You must be signed in to change notification settings - Fork 278
feat: Implement true_disp calculation in ctapipe-process + test #2951
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
Open
koopatroopa787
wants to merge
26
commits into
cta-observatory:main
Choose a base branch
from
koopatroopa787:feature-true-disp-calc
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 4 commits
Commits
Show all changes
26 commits
Select commit
Hold shift + click to select a range
5845e78
feat: Implement true_disp calculation in ctapipe-process + test
koopatroopa787 d775870
docs: Add changelog fragment for #2950
koopatroopa787 bea9764
refactor: Use single true_disp field and shared calculation utility
koopatroopa787 7ed8dec
Refactor true_disp: Move to SimulatedCameraContainer, use shared util…
koopatroopa787 db1e5ac
feat: Calculate true_disp and fix writer bugs
koopatroopa787 348f76e
fix: Robust HDF5TableWriter handling for metadata and rows
koopatroopa787 2f6f3ce
fix: Import Container in tableio.py and finalize writer robustness
koopatroopa787 2bb287f
add lazy property with telescope coordinates as EarthLocation
mexanick 928abe1
add changelog
mexanick 1b013b5
Fix changelog
mexanick df21fae
Improve developer setup instructions in Getting Started guide
yaochengchen dc000e4
add changelog
yaochengchen 035297a
update as reviewer's suggestion
yaochengchen fba4463
Fix trigger check in HDF5EventSource.is_compatible
yaochengchen 9d9e010
add changelog
yaochengchen aa32c83
Fix typos in hillas_reconstructor.py
yaochengchen b157186
Fix a typo in README.md
yaochengchen 6bb1777
Add changelog
yaochengchen 66a2994
Fix a typo in README.rst
yaochengchen b44a018
Fix a typo in README.rst
yaochengchen bd7e32c
Fix a typo in README.rst
yaochengchen d1dc7aa
Fix a typo in hillas.py
yaochengchen 8eeedd3
Modify changelog
yaochengchen 37603e8
Update src/ctapipe/image/hillas.py
yaochengchen ccc3c6d
Update README.rst
yaochengchen 43188e7
Fix missing example blocks in trigger doc
maxnoe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Implement calculation of ``true_disp`` parameters (norm and sign) in ``ctapipe-process`` for simulated events. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
|
|
||
| import numpy as np | ||
| import pandas as pd | ||
| import pytest | ||
| import tables | ||
|
|
||
| from ctapipe.core import run_tool | ||
| from ctapipe.tools.process import ProcessorTool | ||
| from ctapipe.utils import get_dataset_path, resource_file | ||
| from ctapipe.io.hdf5dataformat import SIMULATION_PARAMETERS_GROUP | ||
|
|
||
| def test_true_disp_calculation(tmp_path, dl1_image_file): | ||
| """check true_disp calculation in ctapipe-process""" | ||
| print("DEBUG: Starting test_true_disp_calculation", flush=True) | ||
| config = resource_file("stage1_config.json") | ||
|
|
||
| output_file = tmp_path / "true_disp_test.dl1.h5" | ||
|
|
||
| run_tool( | ||
| ProcessorTool(), | ||
| argv=[ | ||
| f"--config={config}", | ||
| f"--input={dl1_image_file}", | ||
| f"--output={output_file}", | ||
| "--write-parameters", | ||
| "--overwrite", | ||
| ], | ||
| cwd=tmp_path, | ||
| raises=True, | ||
| ) | ||
|
|
||
| # check if fields exist and are not all NaN | ||
| # We need to find a telescope that has events | ||
|
|
||
| with tables.open_file(output_file, mode="r") as testfile: | ||
| # Check if true parameters group exists for at least one telescope | ||
| sim_params_group = testfile.root.simulation.event.telescope.parameters | ||
| assert len(sim_params_group._v_children) > 0 | ||
|
|
||
| first_tel_group = list(sim_params_group._v_children.keys())[0] | ||
|
|
||
| true_params = pd.read_hdf( | ||
| output_file, f"{SIMULATION_PARAMETERS_GROUP}/{first_tel_group}" | ||
| ) | ||
|
|
||
| assert "true_disp" in true_params.columns | ||
|
|
||
| # Check that we have some valid values | ||
| assert np.count_nonzero(np.isfinite(true_params["true_disp"])) > 0 |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
It's good to add this utility, but it should then also be used in the training.
Note that we have two different conventions for computing true disp and this should be configurable here (see the code in the training tool / the
project_dispoption)