ctapipe Bi-Weekly Digest — Activity since 2025-11-23

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

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

  • SimTelEventSource.skip_r1_calibration should still allow to select gain #2895 [maxnoe] enhancement

    Please describe the use case that requires this feature. If skip_r1_calibration is set, also the gain selection is...

  • Incorrect intensity scaling in toymodel due to unit mismatch #2896 [shahjahanIqbal] bug

    Describe the bug The toymodel.ImageModel.expected_signal multiplies the PDF (1 / self.unit^2) with the...

    • maxnoe: Thanks for reporting this issue. The fix is pretty simple, replace the .value with...

  • Add references to calibpipe for camera calibration to the docs #2899 [maxnoe] documentation

    Should be added to the docs of the monitoring source to let people know how to produce camera calibration...

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

  • CI with astropy 7.2rc1 #2884 [maxnoe] no-changelog-needed

    • maxnoe: astropy 7.2 was released and is being used in the normal CI now

  • Fixed bug #2896 #2897 [shahjahanIqbal]

    Intensity scaling issue due to unit mismatch

  • Add possibility to produce DL3 with ctapipe #2727 [mdebony]

    The purpose of this PR is to add support for the creation of DL3 file in ctapipe. The current output format is the...

    • maxnoe: obs_all_info_table["subarray_pointing_lat"].quantity[i] no need to multiply by units

    • maxnoe: You can iterate over the rows of the table and if you convert it to QTable, the items will be...

    • maxnoe: Suggested change class...

  • Fix PendingDeprecationWarning for docutils arg #2860 [maxnoe] no-changelog-needed

    Fixes: src/ctapipe/core/tests/test_component.py::test_component_html_repr ...

    • kosack

  • Support python 3.14, require python >= 3.12 #2855 [maxnoe]

    Tests are passing with numba beta version. No changes needed in ctapipe itself. I suggest to wait for the numba...

    • kosack

    • mexanick

  • More ImPACT code cleanup #2705 [gschwefer]

    Hi everyone, I finally got around to clean up the ImPACT code a bit more. This PR has 3 major changes: The paths to...

    • Tobychev

      • Tobychev: It seems a bit late to check if the template is compatible with the subarray only towards the...

        • Tobychev: Ok, if a pickle requires a full deserialisation to access the keys then this is fine. We should...

    • maxnoe: @kosack @ParsonsRD the static analysis was failing with lots of duplicated code and too little...

    • maxnoe: Suggested change ...

    • maxnoe: Suggested change ...

    • kosack

      • kosack: I would just remove this fallback (see prev comment), and just require that the user use a...

      • kosack: I think that is a bad idea, since it means if somebody forgets to set a correct value, the...

    • kosack: Also, please rebase this on the main branch, since the reason the tests are not all passing is...

    • gschwefer: Thanks for the review @maxnoe @kosack, I'll have another look

  • Flatfielding factor instead of absolute & relative factor for amplitude correction #2433 [StFroese]

    closes #1397

    • Hckjs: can be closed due to #2802, right?

  • Add a StereoDispCombiner #2731 [Hckjs] enhancement algorithm module:reco

    This is basically the StereoCombiner already implemented here in magic-cta-pipe. It combines the mono DISP...

    • Hckjs: combiner_theta2_reco_lon_lat_zen_20_az_0_ac_full_array.pdf combiner_irfs_sens_zen_20_az_0_ac_full...

    • maxnoe: .to_value(u.deg)

    • maxnoe: compute abs(disp) once, re-use

    • maxnoe: I'm not quite sure I understand the logic of weighting distances, which should be related to the...

    • maxnoe: Sorry for the above message, I forgot about the comment of near-parallel high energy showers. So...

    • maxnoe: Could you also maybe expand a bit what datasets you are using here and how they have been...

    • Hckjs: Algorithm: The phrase of "weighting" the distances was maybe a bit misleading. The idea was,...

  • Add full cut optimization as introduced in pyirf 0.13 #2789 [LukasBeiske]

    This changes the PointSourceSensitivityOptimizer to use the full cut optimization introduced in pyirf 0.13. The...

    • Hckjs: gh_cut_efficiency_step=0.02 theta_cut_efficiency_step=0.02 If a smaller step is important to...

    • Hckjs: If i understand correctly, the gh cut optimization: first calculates (initial) theta cuts based...

    • maxnoe: @Hckjs This is correct yes. One could probably get around this by optimizing again at least once...

    • maxnoe: I just noticed this (similar for the full optimization): I didn' realize that plot was for the...

    • LukasBeiske: I just noticed this (similar for the full optimization): I didn' realize that plot was for the...

  • hstack the HDF5 monitoring files #2900 [TjarkMiener]

    Currently in draft mode to gather some early feedback on API design on the merger components. Aiming to: closes...

    • maxnoe: The diff is rather large, could you maybe summarize the changes so we can agree on the concepts?...

    • TjarkMiener: The diff is rather large, could you maybe summarize the changes so we can agree on the...

    • TjarkMiener: @maxnoe I think I managed to include the above functionality by creating an internal option. I...

  • Fixed bug #2896 #2898 [shahjahanIqbal]

    Intensity scaling issue due to unit mismatch The toymodel.ImageModel.expected_signal multiplies the PDF (1 /...

    • maxnoe: Thanks! Could you add a changelog snippet in docs/changes/2898.bugfix.rst? Something like: Fix...

    • maxnoe: Looking a bit deeper into the code, this line is in the abstract baseclass. But this interface...

      • maxnoe: @shahjahanIqbal See this original comment here

    • maxnoe: Same as above. pdf should have units of per area or per solid angle.

      • shahjahanIqbal: Something like this? return u.Quantity(self.trans_dist.pdf(trans) * self.long_dist.pdf(long),...

      • maxnoe: yes, exactly. (just FALSE → False)

      • shahjahanIqbal: I've added this line to the code too

    • maxnoe: I'll approve as the bugfix here, but we should probably update the pdf methods to properly set the units.

    • maxnoe: @shahjahanIqbal Here, we should actually do: return u.Quantity(self.dist.pdf(X),...

      • shahjahanIqbal: Okay. Will make the change

    • maxnoe

    • maxnoe: @shahjahanIqbal I think you only commited partial changes for the full fix of the units. We need...

    • maxnoe: FALSE → False

      • shahjahanIqbal: Didn't notice that. Made the change.

    • maxnoe: @shahjahanIqbal I think you missed at least one toymodel, the RingGaussian. You need to make the...

    • maxnoe: You can run the tests locally, see...

    • maxnoe: See especially the section about running pre-commit

    • kosack: Should just delete this commented-out code if it is no longer used. Otherwise, looks good!

      • shahjahanIqbal: I just ran the test codes. Looks like the units raise an error in the expected_signal() method....

      • maxnoe: This is because you are now mixing the two possible solutions we discussed. You need to: either...

    • maxnoe: Since you now made changes to the pdf functions, the correct fix is here is the alternate...

      • shahjahanIqbal: Made the change

      • shahjahanIqbal: @maxnoe, this is causing a unit conversion error. I believe the earlier return statement already...

      • maxnoe: The pdf and the pix_area are stripped of units. Because you made two changes which are mutually...

    • maxnoe

    • maxnoe: Thanks a lot! Looks good now.

    • shahjahanIqbal: I'm seeing a 20 warnings while running the test. 225 passed, 20 warnings in 47.05s. Is this alright?

    • maxnoe: I will check in more detail once the CI is through here, but yes, unfortunately we are currently...

    • shahjahanIqbal: Okay! I missed running the make doc in the last commit. Sorry about that. I'll just do it right now

    • maxnoe: @shahjahanIqbal no need, the CI for the documentation already passed:...

    • kosack

  • attach monitoring data to hdf5 file #2901 [TjarkMiener]

    This PR tweaks the merger component to add functionalities listed in #2900 (comment) closes #2890 closes #2862

    • mexanick

      • mexanick: Suggested change def...

      • mexanick: Suggested change ...

        • TjarkMiener: This is quite messy. I will think about another solution. The current code is frankensteining...

        • maxnoe: As I have commented before, I am opposed to including the trigger table in the monitoring files...

        • TjarkMiener: Yes, I agree with this, but I can understand that it makes life easier for the qualpipe folks...

        • mexanick: In general, I agree with @maxnoe . What we need at the end is the rate for the quality checks....

        • maxnoe: Trigger rate monitoring can also be computed from the files generated by SWAT, which are much...

        • maxnoe: Actually, I think you need to use those, as these are the only source for busy triggers, where a...

        • maxnoe: If it's interesting, we could also directly compute the mean rate in a chunk in the pixel stats tool.

        • mexanick: no, SWAT-generated files are best for this. This is a qualpipe business and indeed this is much...

        • maxnoe: These are the DL0/Event/Subarray and DL0/Event/Telescope/Trigger data models in the DL0...

      • mexanick: Suggested change _ =...

      • mexanick: I think this is wrong and we should not allow this. Also, in this way you allow to merge...

        • TjarkMiener: Yes, good point. But the simulation is here the config trait and not the file origin itself. I...

        • TjarkMiener: I added a check for the same data category and check here if it is simulations.

        • mexanick: I still think that simulations should have the same obs ids for matching calibrations (and for...

        • maxnoe: No, that won't fly as at least for now, these are created in separate runs. What determines if...

        • TjarkMiener: Also for each simulation production we need a couple of thousands calibration events (just...

        • maxnoe: The events runs are simulated in bunches which share the same instrument seed. This was done to...

      • mexanick: Why is it here and not under the condition of line 274?

        • TjarkMiener: Not sure if I follow the comment correctly since line 274 moved due to the latest commits...

        • mexanick: I mean this block, right above: if self._n_merged == 0: self.meta =...

      • mexanick: I think we should produce new files. And in case of absence of event types the tool should...

        • TjarkMiener: Yes, once this PR is close to be finalised, I will produce new test files with interface tag i0.2.0

        • TjarkMiener: @maxnoe should we maybe move the data format version to v8.0.0 since now we can have the...

created at 2025-12-07 06:34 by dinghy 1.4.1.