Skip to content

Calculate and store true_disp per-telescope when running ctapipe-process#3003

Draft
Copilot wants to merge 10 commits into
mainfrom
copilot/add-true-disp-for-ctapipe-process
Draft

Calculate and store true_disp per-telescope when running ctapipe-process#3003
Copilot wants to merge 10 commits into
mainfrom
copilot/add-true-disp-for-ctapipe-process

Conversation

Copy link
Copy Markdown

Copilot AI commented May 15, 2026

  • Add TrueDispContainer class to containers.py with norm and sign fields (prefix true_disp)
  • Add true_disp field to SimulatedCameraContainer in containers.py
  • Calculate true_disp_norm and true_disp_sign in image_processor.py when use_telescope_frame=True and simulation data is available
  • Write true_disp container to simulation/event/telescope/parameters in datawriter.py
  • Update hdf5eventsource.py to read back true_disp fields when loading existing HDF5 files
  • Add test coverage using synthetic data only (no network access required)
    • test_true_disp_calculation: verifies norm ≥ 0 and sign ∈ {-1, 0, 1}
    • test_true_disp_requires_telescope_frame: verifies NaN defaults when use_telescope_frame=False
    • test_true_disp_no_simulation: verifies graceful handling of missing shower info
  • Add docs/changes/3003.feature.rst changelog entry
  • Fix ruff formatting issues
  • Clamp altitude values to [-90, 90] deg before creating AltAz coordinates
  • Fix docs build: add traitlets.traitlets.K and traitlets.traitlets.V to nitpick_ignore in docs/conf.py to suppress pre-existing upstream warnings
  • Update docs/user-guide/data_format/index.rst to document TrueDispContainer in the simulation parameters table entry

@STSpencer
Copy link
Copy Markdown
Contributor

For reference this is a test. There is an ongoing discussion within CTAO about whether github copilot or codex represents better value for money for the observatory for agentic coding. One of the key issues is whether github integration such as this is worth double the cost. Let's see how well copilot does.

Copilot AI and others added 4 commits May 15, 2026 19:24
- Add TrueDispContainer class to containers.py with norm and sign fields
- Add true_disp field to SimulatedCameraContainer
- Calculate true_disp in ImageProcessor._calculate_true_disp when use_telescope_frame=True
- Write true_disp to simulation/event/telescope/parameters in DataWriter
- Update HDF5EventSource to read back true_disp fields
- Add tests for the new functionality in test_image_processor.py"

Agent-Logs-Url: https://github.com/cta-observatory/ctapipe/sessions/63cfb286-711f-4644-aa96-b0acb650a6a5

Co-authored-by: STSpencer <31512502+STSpencer@users.noreply.github.com>
…eview

- Remove duplicate test functions in test_image_processor.py
- Remove unused imports in test file
- Update TrueDispContainer.sign field description to mention sign can be 0

Agent-Logs-Url: https://github.com/cta-observatory/ctapipe/sessions/63cfb286-711f-4644-aa96-b0acb650a6a5

Co-authored-by: STSpencer <31512502+STSpencer@users.noreply.github.com>
Copilot AI changed the title [WIP] Add true_disp normalization and sign fields in ctapipe-process Calculate and store true_disp per-telescope when running ctapipe-process May 15, 2026
Copilot AI requested a review from STSpencer May 15, 2026 19:31
@STSpencer
Copy link
Copy Markdown
Contributor

@copilot Use existing test files available within the ctapipe framework to evaluate this work. Do not attempt to connect to the DESY cluster

…/event

The new true_disp tests now use a synthetic rectangular camera geometry
and synthetic event data that can be run without any network access.
This eliminates the need to connect to the DESY cluster during testing.

Agent-Logs-Url: https://github.com/cta-observatory/ctapipe/sessions/140fa9a8-8f5f-4f16-ae78-93155d5455f3

Co-authored-by: STSpencer <31512502+STSpencer@users.noreply.github.com>
Copy link
Copy Markdown
Author

Copilot AI commented May 15, 2026

@copilot Use existing test files available within the ctapipe framework to evaluate this work. Do not attempt to connect to the DESY cluster

The three new tests (test_true_disp_calculation, test_true_disp_requires_telescope_frame, test_true_disp_no_simulation) have been rewritten to use fully synthetic data — a rectangular camera geometry with a 1m focal length, built entirely in-memory without any network access. They pass locally without attempting to connect to the DESY cluster (commit ef297f0).

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • minio-cta.zeuthen.desy.de
    • Triggering command: /home/REDACTED/.local/bin/pytest pytest src/ctapipe/image/tests/test_image_processor.py -x -v conf�� e/.git -disp-for-ctapipHEAD ease user.email (dns block)
    • Triggering command: /home/REDACTED/.local/bin/pytest pytest src/ctapipe/image/tests/test_hillas.py -v --ignore-glob=*test_image_processor* -k not gamma_prod5 and not gamma_test e/ it (dns block)
    • Triggering command: /home/REDACTED/.local/bin/pytest pytest src/ctapipe/image/tests/ -k not (test_image_processor[TailcutsImageCleaner] or test_image_processor[NSBImageCleaner] or test_image_processor[MARSImageCleaner] or test_image_processor[FACTImageCleaner] or test_image_processor[TimeConstrainedImageCleaner] or test_image_processor_camer conf�� --local gpg.program tnet/tools/git (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@STSpencer
Copy link
Copy Markdown
Contributor

@copilot Address failing checks

Copy link
Copy Markdown
Author

Copilot AI commented May 15, 2026

@copilot Address failing checks

The failing check was the changelog CI — it required a docs/changes/3003.feature.rst entry. Added in commit f54adb4.

@STSpencer
Copy link
Copy Markdown
Contributor

@copilot CI/lint still failing

Copy link
Copy Markdown
Author

Copilot AI commented May 15, 2026

@copilot CI/lint still failing

Fixed in commit d6607a8ruff format was flagging formatting issues in image_processor.py and test_image_processor.py. Both files have been reformatted to pass the linter.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • minio-cta.zeuthen.desy.de
    • Triggering command: /usr/bin/python python -m pytest src/ctapipe/image/tests/test_image_processor.py -x -q (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@STSpencer
Copy link
Copy Markdown
Contributor

@copilot Continue to address failing checks

…oating point values

Agent-Logs-Url: https://github.com/cta-observatory/ctapipe/sessions/8d9a62d3-f9d7-4fc0-b9a9-a11f49e60d38

Co-authored-by: STSpencer <31512502+STSpencer@users.noreply.github.com>
Copy link
Copy Markdown
Author

Copilot AI commented May 15, 2026

@copilot Continue to address failing checks

Fixed in commit 57ddb1c. The test failure was in _calculate_true_disp: pointing_alt (and shower.alt) could be marginally above 90° due to floating point, causing astropy's AltAz to raise ValueError. I now clamp both values to [-90, 90] degrees before constructing the coordinate frame.

The docs build failure (8 warnings from traitlets.traitlets.K/V) is pre-existing on main and unrelated to this PR.

@STSpencer
Copy link
Copy Markdown
Contributor

@copilot Address failing docs checks and ensure this PR contains adequate documentation

… data format docs for TrueDispContainer

Agent-Logs-Url: https://github.com/cta-observatory/ctapipe/sessions/128715bf-16e5-4cb4-843d-c258647fd041

Co-authored-by: STSpencer <31512502+STSpencer@users.noreply.github.com>
Copy link
Copy Markdown
Author

Copilot AI commented May 17, 2026

@copilot Address failing docs checks and ensure this PR contains adequate documentation

Addressed in commit 5c53271:

  1. Docs build fix: Added traitlets.traitlets.K and traitlets.traitlets.V to the nitpick_ignore list in docs/conf.py. These 8 warnings (4 Dict methods × K/V) were the sole cause of the docs build failure — they are a pre-existing upstream traitlets issue, now suppressed the same way other traitlets references are handled.

  2. Feature documentation: Updated docs/user-guide/data_format/index.rst to document the new TrueDispContainer in the simulation parameters table (/simulation/event/telescope/parameters), with a description explaining its purpose for training and evaluating angular reconstruction algorithms.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • docs.astropy.org
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • docs.bokeh.org
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • docs.cython.org
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • docs.pytest.org
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • docs.python.org
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • docs.scipy.org
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • ipywidgets.readthedocs.io
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • joblib.readthedocs.io
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • matplotlib.org
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • minio-cta.zeuthen.desy.de
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • numba.readthedocs.io
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • numpy.org
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • pandas.pydata.org
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • psutil.readthedocs.io
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • pyirf.readthedocs.io
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • scikit-hep.org
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • scikit-learn.org
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • setuptools.pypa.io
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • traitlets.readthedocs.io
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)
  • www.pytables.org
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build -q (dns block)
    • Triggering command: /usr/bin/python python -m sphinx -W --keep-going -b html . /tmp/docs_build (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@STSpencer
Copy link
Copy Markdown
Contributor

@copilot Address failing checks

Copy link
Copy Markdown
Author

Copilot AI commented May 17, 2026

@copilot Address failing checks

The latest CI run on commit 5c53271 shows docs ✅ and lint ✅ both passing. The single failing job (tests Linux 3.14) was canceled by a runner shutdown signal at 88% completion — no test failures, just an infrastructure event. All other test jobs were cascaded as canceled as a result. This is not caused by the code changes in this PR.

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.

Calculate true_disp when running ctapipe-process

2 participants