ctapipe Bi-Weekly Digest — Activity since 2026-02-10

» cta-observatory/ctapipe | cta-observatory/ctapipe

• cta-observatory/ctapipe repo issues [back to top]

  • Implement loading of instrument configuration from service data given a subarray id #2946 [maxnoe] enhancement

    Please describe the use case that requires this feature. We need to load the instrument configuration by subarray id...

    • mexanick: @maxnoe shall we add simplified optics tables, that would be mostly placeholders now, but we can...

  • Calculate true_disp when running ctapipe-process #2950 [STSpencer] enhancement good first issue

    Please describe the use case that requires this feature. Currently (as far as I can tell), ctapipe-process does not...

    • maxnoe: If you just want to have a quick look at performance figures, the true disp is stored as part of...

  • HillasIntersection with weighting = "hess" does not work #2805 [calispac] bug

    Describe the bug The weighting method 'hess' of the HillasIntersection reconstructor is not implemented although it...

    • maxnoe: The method has been removed in #2880

  • Add "column" mode to EventPreprocessor: keep but mark events that don't pass qualityquery #2954 [kosack] enhancement

    Please describe the use case that requires this feature. Currently the EventPreprocessor, #2928 (and the older...

  • Cuts and X_max to EventPreprocessor #2953 [kosack] enhancement

    Please describe the use case that requires this feature. Once #2928 is merged, the following features were requested...

  • R1/DL0 telescope event type is also stored in trigger. #2952 [maxnoe] bug

    Describe the bug We store the R1/DL0 telescope event type in three containers, R1, DL0 and TelescopeTrigger. To...

    • kosack: Having it both in R1 and DL0 is fine, since those are mean to have similar info. So I guess...

    • maxnoe: This issue is more about having it also at the trigger container. The field in R1 and DL0 is...

    • maxnoe: We should actually replace the event_type in the trigger container with the trigger type from...

• cta-observatory/ctapipe repo pull_requests [back to top]

  • Force absences of units for skewness and excess_kurtosis #2732 [mexanick] fix

    Explicitly convert (dimensionless) skewness and excess_kurtosis to values Closes #2730

  • Improve statistics aggregators #2848 [mexanick]

    Relax requirements on the data shape (aggregate arbitrary arrays on axis 0) Add option to aggregate statistics by...

  • Only set coverage options in coverage job #2882 [mexanick] fix maintenance no-changelog-needed

    coverage-7.11.1 changed the way how the measurement core setting is treated (changelog): Fix: if the measurement...

  • Add more calibration and monitoring groups to HDF data format #2881 [mexanick]

  • allow gain selection when skipping R1 calibration in SimTelEventSourve #2943 [TjarkMiener]

    • maxnoe: Instead of duplicating the code here, please just factor the gain selection out of the...

    • maxnoe: We have a caching _get_pixel_index helper (actually in two positions in the code to avoid...

    • TjarkMiener: I need to understand this change, so I marked the PR in draft mode.

      • TjarkMiener: Related to changes#r2788135455

    • TjarkMiener: gain selection should be done on the r0_waveform

    • mexanick

  • Allow filtering by telescope event type in software trigger #2941 [maxnoe]

    From a discussion with @mdpunch (cc @mdebony), the nectar cam people would like to analyze the mono performance of...

    • kosack: If I can, I can't figure out how to do it... @mdpunch You just go to the "Files Changed" tab at...

    • mdpunch: See previous comment. All good for me! Thanks @maxnoe !

    • mdebony

  • EarthLocation of the telescopes in subarray #2947 [mexanick]

    add lazy property with telescope coordinates as EarthLocation

    • maxnoe

    • TjarkMiener

    • mexanick: @maxnoe does this need anything else, or could be merged?

    • maxnoe: Review requested from @kosack

    • kosack

  • Improve developer setup instructions in Getting Started guide #2948 [yaochengchen]

    Clean up the Getting Started developer guide by fixing typos, correcting incorrect Git commands, and aligning...

    • yaochengchen: Hi, I am a member of the CTA Consortium and would like to contribute more regularly to...

    • kosack: Looks good otherwise! Thanks

      • kosack: later the document refers to the cta-dev environment (which is the name inside environment.yml),...

        • yaochengchen: @kosack Thanks for pointing this out, sorry about that, I have dropped "-n ctapipe=dev" as you suggested.

    • kosack

    • maxnoe

    • maxnoe: Hi, I am a member of the CTA Consortium and would like to contribute more regularly to...

    • yaochengchen: Hi, I am a member of the CTA Consortium and would like to contribute more regularly to...

  • feat: Implement true_disp calculation in ctapipe-process + test #2951 [koopatroopa787]

    Description This PR implements the calculation and storage of true_disp parameters (norm and sign) in...

    • yaochengchen: hi @koopatroopa787 , nice timing 🙂 I was actually working on something similar, but you got...

    • maxnoe: Please remove this unrelated change from this pull request.

      • koopatroopa787: Hii, On Windows, I found that traitlets (or the ctapipe wrapper) struggles to validate absolute...

      • maxnoe: We at the moment explicitly do not support and do not test on windows.

    • maxnoe: disp is not an image parameter and does not belong in this group.

    • maxnoe: Please do not put inline computations inside of the process tool. The CLI is a high-level...

    • maxnoe: We store the predicted disp as a single column, not in separate norm and sign columns. The true...

    • maxnoe: This is not really an image parameter, so should not be part of this data structure. It rather...

      • maxnoe: It can also directly use the DispContainer.

    • koopatroopa787: Hey @yaochengchen and @maxnoe , thanks for the help with this! I'm still getting the hang of...

    • maxnoe: It's good to add this utility, but it should then also be used in the training. Note that we...

  • Test with numba 0.64rc1 #2940 [maxnoe]

  • Extend MonitoringSource interface #2933 [mexanick] new functionality module:IO

    Prepare the interface part to add the DL0/Monitoring source plugin (will come in a separate repository). Therefore,...

    • TjarkMiener: @mexanick Could we switch the base to polish_psfmodel? Then, I'd could use this branch for...

    • mexanick: @mexanick Could we switch the base to polish_psfmodel? Then, I'd could use this branch for...

  • Fix trigger check in HDF5EventSource.is_compatible #2949 [yaochengchen]

    Fix trigger compatibility check has_trigger in HDF5EventSource.is_compatible() incorrectly...

    • maxnoe

    • kosack

  • Implement service data loading #2942 [mexanick]

    Add from_service_data() classmethod to construct SubarrayDescription from service data files using subarray...

    • mexanick: @maxnoe I think I've addressed all your comments apart from the camera type inference. I think...

    • kosack

      • kosack: do you really need a dict[EarthLocation]? If the conversion is slow, why not just have this...

        • mexanick: this was exactly what I originally implemented, and @maxnoe asked to do it this way :)) I think...

      • kosack: should mention here that you also do the mapping of id to official telescope name like "LSTN-001".

      • kosack: Is the table being loaded here some standard from CTAO? Or is this a new format? If new, I...

        • maxnoe: It's a table I produced for https://github.com/cta-observatory/ctapipe_io_zfits from the...

        • maxnoe: the alternative would be to use the official table and add pyproj as an optional dependency to...

        • maxnoe: In the end, I think this needs to come from SOSS already including all three sets of coordinates...

        • mexanick: I also think it should come from SOSS, and I would prefer to use a table now, instead of...

      • kosack: Is there really a good case for relying on from_name() giving the right information? It was...

        • mexanick: I believe we need a way to get the subarray from service data, as we do not always have events...

        • maxnoe: or should we not just stick with the "new" way of working (subarray comes from the...

        • maxnoe: We also need a common way for loading it, as it is needed in multiple places. We are only...

    • kosack: I think in general I'm missing the use case here for the parts on loading the subarray from...

    • mexanick: I think in general I'm missing the use case here for the parts on loading the subarray from...

    • maxnoe: We had abandoned that API in favor of requring the logic of creating a SubarrayDescription in...

    • maxnoe: I could imaging that we do something like this now in Tool.setup: self.source =...

    • maxnoe: I could imaging that we do something like this now in Tool.setup: Discussed: we still want to...

    • kosack: any reason you use shutil instead of ...

      • mexanick: Path.copy() was only added in python 3.14, and I believe we should support 3.12 and 3.13

    • mexanick: @maxnoe @kosack ready for review

    • maxnoe: Let's not say "CTAO data model format" until there actually is one. At the moment, this is...

    • maxnoe: Can we add an instrument.meta.json or something similar at the root level here that contains at...

    • maxnoe: Suggested change -...

    • maxnoe: I'd not write "symlinks" here as if that would be prescriptive. Maybe: "Instrument description...

    • maxnoe: Why is this a table with one row? That seems weird in ecsv. We plan to use this for mirror...

      • mexanick: this looks weird because it is current optics.ecsv table (with one row per telescope) split into...

      • maxnoe: But you don't need to modify the optics description code, right? There is no...

    • maxnoe: How is not finding a telescope description for a given subarray only a warning? This needs to be...

    • maxnoe: You can use the monkeypatch fixture for this using monkeypatch.setenv, it will restore automatically.

      • maxnoe: https://docs.pytest.org/en/stable/reference/reference.html#pytest.MonkeyPatch.setenv

      • mexanick: thanks, done!

    • maxnoe: Final comment on general aspects: ctapipe-dump-instrument should support this format as output...

    • mexanick: Final comment on general aspects: ctapipe-dump-instrument should support this format as output...

    • maxnoe: Great, thanks! I'll use that to have a look at the resulting structure for our simulations and...

  • Feature: EventPreprocessor #2928 [kosack]

    Introduces an EventPreprocessor class that can generically transform an event table by applying the following...

    • maxnoe: @mdebony This is intended for DL2 to DL3, so please take a look here. I think it would be good...

    • Voutsi: I think there is a typo here: true_fov_lon shouldn't be reco_fov_lon?

      • kosack: Thanks! not sure how I missed that. I even made plots of the results. But I think that was...

    • Voutsi: Also here, I think reco should be replaced by true

    • mexanick

      • mexanick: there's no dl2_simulation feature set, shall it be added?

        • kosack: I just forgot to rename it in the changelog - it's the one that is implemented so far, for use...

      • mexanick: Shall we perhaps use a registry to allow extension of the feature sets? e.g. class...

        • kosack: Sounds reasonable! That is nice in that it separates these pre-configured options from the core...

        • kosack: implemented. It's indeed much cleaner.

    • maxnoe

    • mexanick: LGTM!

    • Voutsi

    • mdebony: Seems very usefull for DL3 production, I've added a small suggestion in the comments.

      • mdebony: Would be nice if they are possibility to add input kwargs of to function and transmit it to the...

        • kosack: If that's ok, and you think the general structure is fine please accept the review here.

        • kosack: Thanks. I think then I'll merge what is here, and add this feature (and the gamma/hadron cuts...

  • Polar PSFModel #2945 [TjarkMiener]

    • maxnoe: This makes it impossible to get the PSF prediction in TelescopeFrame, i.e. LAT/LON in m, is that...

      • mexanick: I actually want to make it only working with lat/lon, and better in angular units, to make more...

    • maxnoe: Why is the pixel width a configuration parameter and not taken from the camera geometry in the...

      • maxnoe: PSFModel is a TelescopeComponent, so it always has access to the subarray info.

      • mexanick: also looks like it is only used in pdf production, that should probably just take a camera...

      • maxnoe: pdf should get a tel_id as first argument I think.

      • maxnoe: (in the abstract interface)

      • mexanick: yes, I also meant changing the abstract interface. Indeed, tel_id would be more appropriate than...

      • TjarkMiener: Sounds good

    • TjarkMiener: Should we make those guys telescope-specific? Currently we need to write configs for each tel type.

      • mexanick: yes, all these parameters are telescope-specific and should be set as TelescopeParameters

    • mexanick

      • mexanick: Suggested change Polish the PSFModel...

      • mexanick: I'd prefer something like this: asymmetry_params = TelescopeParameter( ...

        • maxnoe: Why? The other names are much clearer and actually document what each parameter is

        • mexanick: I'd say too many details.. ) but if you find it better like that, I'm fine with it too.

    • maxnoe: You made larger changes now that should be reflected in the changelog

    • mexanick

    • kosack: I don't see the attributes asymmetry_params, radial_scale_params or phi_scale_params in the...

      • TjarkMiener: Done in 90eda9c

    • kosack: I think it would be good to give defaults for all the parameters, since running this now without...

    • mexanick: @kosack we only know defaults for LST, but not for other telescopes. With the transition to...

    • kosack: @kosack we only know defaults for LST, but not for other telescopes. With the transition to...

    • maxnoe: Maybe a better way to say it: in all ctapipe components used now, config parameters are always...

    • kosack: In this particular case here, I'm not quite sure what to do. I think providing defaults might be...

    • mexanick: In this particular case here, I'm not quite sure what to do. I think providing defaults might be...

    • kosack: It is in the "instrument" part because it is indeed a characteristic of an instrument, however...

    • maxnoe: You could mimic that with a psf_model StringTelescopeParameter in your parent class, We already...

    • mexanick

    • kosack: Looks ok now, thanks. one question: I was trying to use it to visualize a distorted PSF, but I...

    • mexanick: @TjarkMiener please check/update default parameters after switching to lon/lat, we should get...

    • kosack: If it helps, this is what I plotted above: def plot_psf(psf: PSFModel, lat0=0 * u.deg, lon0=0 *...

    • mexanick

    • TjarkMiener: @kosack @mexanick I pushed a fix for the observed issues. I'm now obtaining reasonable PSFs I'd...

    • maxnoe: Shouldn't this be "offset" and not "center"? It's the constant term in the polynomial, right?

    • maxnoe: Why do we now manually compute a polynomial, when before we were using polyval, which should be...

    • mexanick: @maxnoe I believe the labels and plots are mixed, (just swap HERE <->MAIN or the pairs of plots)...

    • maxnoe: I believe the labels and plots are mixed, Yes, fixed now

    • maxnoe: There still seems to be something off here. I get very different results here than on main for...

    • maxnoe: Why remove the _cartesian_to_polar helper and put the same code inline?

    • maxnoe: This is not needed, the transformation CameraFrame ↔ TelescopeFrame does not require pointing...

    • maxnoe: This should be done once in __init__ and only if the given subarray does not already contain...

    • TjarkMiener: Thanks @maxnoe! I implemented the code suggestions. I'd need to have a closer look on what is...

created at 2026-02-24 07:06 by dinghy 1.4.1.