ctapipe Bi-Weekly Digest — Activity since 2024-11-05

• cta-observatory/ctapipe repo issues

  • Make some dependencies optional #2638 [maxnoe]

    To enable some more minimal installations of ctapipe, e.g. for ACADA SAG, we should make at least the following...

  • changelog CI job fails #2640 [mexanick] bug

    Describe the bug An explicit check on the changelog records on their types fails to expand the filename patterns...

  • Add API to `EventSource` for reading the reference metadata #2645 [maxnoe] enhancement

    Please describe the use case that requires this feature. Add the moment, the Provenance system tries to read the...

  • Document the layers of the ctapipe API better #2646 [kosack] documentation

    This was noted in discussions with ACADA's SAG-RECO team who are considering using some parts of ctapipe. It was...

    • kosack: The summary is maybe more that we need a proper architecture section of the docs, with some text...

  • Add psf model to `InstrumentDescription` #2647 [ctoennis] enhancement

    The InstrumentDescription model should have a place to store the upcoming PSFModel in PR #2643 for each telescope,...

  • shower_distribution not propagated by HDF5EventSource #2632 [kosack] bug

    Describe the bug The /simulation/service/shower_distribution histograms are written if you read a simtel file with...

  • Move data volume reduction from image to own submodule #1394 [maxnoe]

    This is it's own thing, it should either be it's own submodule or a part of calibration. I lean towards it being it...

  • Support reading sim_telarray DST0 files #1777 [ParsonsRD] new functionality

    Hi all, The reading of the DST0 files (essentially dl1 data level) produced by Konrad's sim_telarray is currently...

  • remove support for calculating Hillas parameters in CameraFrame #2061 [kosack] refactoring

    see older issue: #1983 Now that everything works consistently in the TelescopeFrame for reconstruction, we should...

  • Machine Learning Tutorial #2135 [nbiederbeck] enhancement documentation

    Since we now have the machine learning tools, we need to show users how to work with them. They should be (at least)...

  • Make QualityQuery a TelescopeComponent and list of checks a TelescopeParameter #2158 [maxnoe] enhancement

    Please describe the use case that requires this feature. To support different cuts per telescope type. Describe the...

  • QualityQuery statistics are written to hdf5 file without the <kind> group #2177 [maxnoe] bug IO hdf5 data format

    Describe the bug ❯ h5dump -n gamma-diffuse_with_images_00.dl2.h5 | grep statistics dataset ...

  • Tool Documentation needs updates #2255 [kosack] documentation

    the tools page only lists a few of the existing tools and includes the old muon tool that no longer exists (may be...

  • Should we deprecate changed config options? #2521 [kosack]

    In #2520, I changed a bunch of config option names, which will break existing config files. Maybe before release...

  • Pointing information is not read anymore from files produced with ctapipe versions older than 0.21: #2651 [maxnoe]

    Due to the changes in the HDF5EventSource here: #2438 telescope pointing information is not read anymore from files...

    • maxnoe: This affects for example the public dl1/dl2 dataset, which was produced with 0.17

  • Add nice __repr__ s to the reference metadata objects #2649 [maxnoe] enhancement

    Please describe the use case that requires this feature. The Reference metadata objects currently do not have...

    • kosack: Some have reprs, but it is not consistent. Could just use the _to_dict(self) helper function in...

• cta-observatory/ctapipe repo pull_requests

  • Add Hipparcos as second star catalog, apply space motion #2625 [ctoennis] coordinates calibration new functionality

    This pull request adds a utility to get star positions with proper motion from different catalogues. It allows to...

    • kosack: This doesn't seem to be the case. The function is defined below as: def get_star_catalog( ...

      • mexanick: @kosack yes, you're right. Fixed now.

    • maxnoe

    • maxnoe

    • kosack

  • Improve changelog checks #2637 [mexanick] enhancement fix no-changelog-needed

    Only allow changelog types that are listed in the README Fixes the absence of towncrier changelog record when the...

    • maxnoe

    • Tobychev

  • Test on astropy 7rc1 #2639 [maxnoe]

  • Fix changelog checks #2642 [mexanick] fix no-changelog-needed

    Use bash script instead of file-existence-action, as that action doesn't seem to have an option "check if at least...

    • maxnoe: Couldn't this just be: find docs/changes -type f | grep -P...

      • mexanick: automatically did in pure bash, but this is indeed much neater.

    • maxnoe

    • LukasBeiske

    • Tobychev: Are you absolutely sure that no space after "!" works as you want? The evaluation rules for bash...

      • mexanick: yes. This PR has the "no-changelog-needed" label and passes the changelog check despite it has...

      • maxnoe: This is not bash, it is githubs expression language

  • Refactor how shower distributions are written, write them for all EventSources, implement for HDF5EventSource #2633 [maxnoe]

    Fixes #2632

    • kosack

    • Tobychev

  • New 3D shower model #1996 [StFroese] new functionality reconstruction

    This is just a draft for now to review some ideas. This approach is based on this paper. The shower is basically a...

  • make qualityquery a telescopecomponent #2234 [nbiederbeck]

    Fixes #2158. This is currently ugly, as we now yield from a list with single value. This would need a change to...

  • Drop camera frame #2396 [StFroese] API change refactoring

    As you may remember, I already started this here #2141. A lot happened since the last time (i.e. moving the muon...

  • Fix dtype of tel_id in subarray hdf table #2622 [maxnoe] no-changelog-needed

  • Make some dependencies optional #2641 [maxnoe]

    matplotlib Bokeh eventio iminuit

    • maxnoe: People will get an OptionalDependencyMisisng error now for these: In [3]: s =...

    • maxnoe: And for using EventSource I added special handling for OptionalDependencyMissing: ValueError:...

    • Tobychev: Why strip out matplotlib? Is this to save time on the cluster setup? My first reaction is that...

    • maxnoe: Tests (including the new one only testing the strictly required dependencies) are now...

    • maxnoe: Why strip out matplotlib? Is this to save time on the cluster setup? My first reaction is that...

    • maxnoe: Why strip out matplotlib? Is this to save time on the cluster setup? My first reaction is that...

    • maxnoe: Astropy is currently preparing a PEP for making it possible to have more dependencies by default...

    • kosack: maybe say "EventSources that are installed but need additional packages to be operational..."...

    • kosack

    • Tobychev: What is this disappearing dependency, shouldn't it be mentioned in the changelog?

      • maxnoe: This was an optional dependency of eventio, but is required for a long time now. It's not a...

    • Tobychev

  • Add property to EventSource for support of CTAO reference metadata #2644 [maxnoe]

    Set to false for SimTelEventSource Silences a warning for SimTelEventSource, which at least currently cannot support...

    • Tobychev

    • maxnoe: I think a better solution would be #2648

    • kosack

    • maxnoe: Closing in favor of #2648

  • PSF model #2643 [ctoennis] enhancement calibration new functionality

    I am adding PSF models to ctapipe.image.psf_model. I made a parent class called PSFModel and a PSF model based on...

    • kosack: Quick question: are these models applicable for both gamma-ray and optical PSF? If not, maybe...

    • maxnoe: I think this is purely optical, not IRF. Also, I think the appropriate place would be in...

    • ctoennis: Quick question: are these models applicable for both gamma-ray and optical PSF? If not, maybe...

    • ctoennis: I think this is purely optical, not IRF. Also, I think the appropriate place would be in...

    • kosack: Side note: we should check with what is used in SimPipe's instrument model, to ensure what is...

    • maxnoe: Side note: we should check with what is used in SimPipe's instrument model, to ensure what is...

    • kosack: I think the appropriate place would be in ctapipe.instrument.optics The instrument module is...

    • kosack: could be confusing here... In the instrument model, we use from_name to load an instance of ...

      • ctoennis: I have renamed it for now to avoid confusion. The main purpose of this feature was meant to...

    • kosack: not needed I think. This is not a Component class.

      • ctoennis: i am removing it

    • ctoennis: I think the appropriate place would be in ctapipe.instrument.optics The instrument module is...

    • ctoennis: I think the appropriate place would be in ctapipe.instrument.optics The instrument module is...

    • maxnoe: These parameters need better explanations and possibly units. I suggest to add the relevant...

      • ctoennis: I'll add units where possible, but for the most part they won't have them. I will add the...

      • mexanick: this is an ansatz recursive parametrization (parametrization of the parameters) :) Done...

    • maxnoe

      • maxnoe: double scale

      • maxnoe: some validation should happen here I think

      • maxnoe: Why do we need an "update" function? Do we have to pre-compute expensive operations? Why is...

        • ctoennis: I have right now 2 use cases for this PSF model: processing variance images (e.g. finding stars...

        • ctoennis: i will try and make the documentation more clear.

        • maxnoe: The question is why this point needs to be state on the class and cannot just be evaluated.

        • mexanick: I believe this is a remnant of the debugging code I used during the development... :) @ctoennis,...

        • ctoennis: ok, i'll put it all together then. I am having a little issue with the docustrings i'm sorting...

      • maxnoe: This looks like it should use https://numpy.org/doc/stable/reference/generated/numpy.polyval.html

      • maxnoe: Is there a citable reference for the math here?

        • ctoennis: Here it is: https://www.aanda.org/articles/aa/full_html/2023/11/aa47128-23/aa47128-23.html I...

        • maxnoe: The bib file is...

        • ctoennis: awesome. will do

        • maxnoe: Which makes me realize we didn't add a reference for the hipparcos catalog

        • ctoennis: i can add that.

    • maxnoe: Where do these magic numbers come from? How are you expecting we determine these numbers? Is...

      • ctoennis: I got these numbers from https://gitlab.cern.ch/cta-unige/startracker (line [162...

      • ctoennis: So, when i will use this for extracting stars these parameters will not need to be altered....

      • maxnoe: So, when i will use this for extracting stars these parameters will not need to be altered for...

      • ctoennis: Ah, yes, they need to be configured per telescope.

      • maxnoe: Ok, then I think we should discuss a bit about API. I think, it's natural then to make this a...

      • ctoennis: that makes sense. For the fitting though my idea was to fit the PSF based on images with...

      • ctoennis: I made the PSFModel into a TelescopeComponent. I will need to see how i will handle the fitting...

    • maxnoe: not needed, since identical with baseclass. Docstring should be at the class definition, not __init__.

      • ctoennis: OK, I'll fix it.

    • maxnoe: This needs a docstring so that subclass implementers know what is expected. Also the signature...

    • orelgueta: Side note: we should check with what is used in SimPipe's instrument model, to ensure what is...

  • Irf maker and cut optimiser #2473 [Tobychev]

    As we are planning to produce IRFs on events that were not used to select the GH cut, it makes sense to split the...

    • maxnoe: In #2641, we made some of the ctapipe dependencies optional. I think this should also be the...

  • Stats calc tool #2628 [TjarkMiener] calibration

    This PR adds a generic stats-calculation tool utilizing the PixelStatisticsCalculator. Related #2542

    • mexanick

      • mexanick: Is DL1a/b is "official"? Also, I'd perhaps use generic input_column_name similar to the output one.

        • maxnoe: ToolConfigurationError is raised once the column is not found. Having list of columns seems a...

    • kosack: you need to add TableLoader to this list, i.e. classes = [TableLoader,] +...

    • kosack: will also need to change all instances of self.input_url to be self.input_data.input_url

      • TjarkMiener: Therefore I'd need to init the TableLoarder before performing the check.

      • maxnoe: yes.

    • maxnoe: Currently fails to read prod5 files (which have no EFFECTIVE focal length information). prod 5...

    • kosack: I get a crash here: % ctapipe-calculate-pixel-statistics -i events.dl1.h5 -o stats.h5 ...

      • TjarkMiener: Good point! I do not know why the test pass here. I will include the change.

    • kosack: Currently fails to read prod3 files (which have no EFFECTIVE focal length information). The...

      • kosack: This needs to be configurable, so you need to pass parent=self in the constructor. You should...

        • TjarkMiener: Okay that sounds good!

      • kosack: You should remove this option, and instead use the parameter provided by TableLoader.

      • kosack: Suggested change ("i",...

    • kosack: In the output, how can I tell what column was aggregated? It is always named "statistics" and...

    • kosack: A more general comment: with very minor changes, this could be turned into...

    • kosack: A common error is to have too small a chunk size, but this now results in a very ugly error and...

    • mexanick

    • TjarkMiener: The former (Unexpected exception) should e caught and raises as a ToolConfigurationError, so the...

    • Hckjs: Despite the leading 'decrease' - 'at least' could be a bit ambiguous here. Maybe ...' to a...

    • TjarkMiener: In the output, how can I tell what column was aggregated? It is always named "statistics" and...

    • mexanick: the tool will raise ToolConfigurationError in a few situations. Please test all of them and use...

  • Update / add reprs for reference metadata objects #2650 [maxnoe]

    Fixes #2649

    • Tobychev

      • Tobychev: why the "Z"?

        • maxnoe: ISO8601 timezone code for UTC

      • Tobychev: why the "Z"?

        • maxnoe: ISO8601 timezone code for UTC

        • maxnoe: ah, wait, id. Good catch. thanks

        • maxnoe: copy / paste error

        • maxnoe: Fixed

    • Tobychev

    • kosack

  • Variance calibration #2636 [ctoennis] calibration

    This PR adds a trait to the CameraCalibrator called "image_calibration_type". This trait allows to switch the method...

    • mexanick: A bit weird that you need to set keepdims=False explicitly, but ok.

      • ctoennis: i will check again if i make it work without that

      • kosack: Should work without it: x = np.random.uniform(size=(2,1800, 1000)) In [1]: np.nanvar(x,...

    • mexanick: Add a test with the LST (two gains)

      • ctoennis: ok

      • ctoennis: would it make sense to add a test for other extractor with the LST?

      • kosack: You could use a parametrized test here to test all telescope types. ...

    • mexanick: should be square, not square root

    • mexanick: same as l.300

    • maxnoe: Please describe the actual change, not just say "there are changes". Also there is a verb...

      • ctoennis: i added some explanation

    • mexanick

    • mexanick: Please format according to ReST syntax. This will render as a single line.

      • ctoennis: I changed it

    • maxnoe: I think we can (maybe should) apply this sqrt operation before writing the coefficients....

    • ctoennis: @maxnoe square is correct. If we apply the square operation when writing the calibration...

    • maxnoe: You still need the peak_time check, right?

    • ctoennis: Oh yes, that will be still needed

    • TjarkMiener: I discussed with @mexanick offline yesterday. We might not want to run the tool for computing...

    • mexanick: at least for now, I would avoid recomputation of the calibrations just for variance. With the...

    • kosack: Please use this to check type, not a string comparison (which is slower and makes refactoring...

      • ctoennis: Makes sense, im changing it now

    • kosack: Suggested change if...

    • ctoennis: i will change the test to check all cameras

    • ctoennis: @kosack I used the camera_geometry fixture instead to check for all cameras. In the end I just...

    • ctoennis: I think there is an issue with the recent change to the changelog check (#2637).

    • ctoennis: i switched it to a parametrized test for the different cameras now and caught the other test for...

    • maxnoe: I think this section could use some refactoring to make it easier to follow. I think the logic...

      • ctoennis: Sounds good, I'll get to it when I arrive home

      • ctoennis: i put it in

    • mexanick: the first dimension is n_channels, isn't it?

      • ctoennis: when i look at the waveforms in the ACADA files i have the waveforms for the LSTCam have the...

      • maxnoe: No, (channels, pixels) is the correct shape for a multi-channel image

      • maxnoe: The comment was that you should replace the magic 2 with camera.readout.n_channels I think

      • ctoennis: Ah. Will do.

    • maxnoe: I think you also should add a note that you fixed the CameraCalibrator for extractors which do...

      • ctoennis: I added a note below

    • maxnoe: logically, I'd put the dl1.peak_time is not None check first

    • maxnoe: "will not try to skip shifting" is a wrong. It will skip. And (insert yoda) there is no try..

      • ctoennis: sorry, i'll fix it

    • maxnoe: @ctoennis Could you please rebase / merge with main so that the correct CI definition runs here

    • maxnoe

    • mexanick

    • kosack

  • Chunk interpolation to select calibration data #2634 [ctoennis] calibration new functionality

    I will need a method to select calibration data for the strar tracker. I made some slides to decribe how it is...

    • ctoennis: @maxnoe @kosack Can you have another look if there is something else to be changed?

    • mexanick: @kosack @maxnoe this PR is needed to complete the pointing calibration (for the variance...

    • maxnoe: I think it's a bit weird here to store closures in a dict instead of just calling a method with...

  • Fix CTAO link in docs #2653 [maxnoe] no-changelog-needed documentation-only

    • Tobychev

    • kosack: I pushed a commit with some more general changes to CTA → CTAO (except for places where it...

    • morcuended: Also in docs/index.rst: ...data processing algorithms for the Cherenkov Telescope Array.

    • maxnoe: This could use the correct title and provide a reference. CTAO Software Programming Standards...

      • kosack: Is it public? I guess we can add a link to the private version in any case

      • maxnoe: No, but we have tons of other documents in the bibliography with the link to the computing-info...

      • maxnoe: I'll add the reference

    • kosack

    • maxnoe: @kosack your changes look good to me, but since it's my PR I cannot approve

    • morcuended: Looks good

    • kosack

  • Improve reference metadata handling for EventSource #2648 [maxnoe]

    The recent addition of attaching the reference metadata of input files to the provenance information implemented...

    • kosack: Looks like a good idea. Probably need some tests, particularly to ensure that anyone...

    • maxnoe: anyone implementing an EventSource plugin doesn't forget to add the input file to the...

    • maxnoe: Probably need some tests, particularly to ensure that anyone implementing an EventSource plugin...

    • kosack: I want to support event sources that have multiple input files, as I know those exist (and we'll...

    • maxnoe: I am surprised to hear this... in the original issue #2571 (comment) and the implementation...

    • kosack: I am surprised to hear this... in the original issue #2571 (comment) and the implementation...

    • maxnoe: So I think we need the input product_ids in the local provanance, but not in the Reference...

    • Tobychev

      • Tobychev: Why is the help string to __init__ being removed?

        • maxnoe: Because it had outdated information and __init__ shouldn't be used at all for documentation in...

        • Tobychev: The class docstring is missing the constructor parameters mandated by the styleguide The...

        • maxnoe: I'd rather say that the section is wrongly labeled Attributes instead of parameters...

        • maxnoe: fixed

      • Tobychev: This new return is not mentioned in the class or method doc strings, so they need updating to...

        • maxnoe: This is mostly an internal method to be called when a Tool starts up. It made my linter a bit...

        • kosack: Yes, and in the PR ActivityProvenance is made private in any case.

    • Tobychev: Added twice? You meant attributes I guess?

      • maxnoe: Fixed, please re-approve

    • Tobychev

    • kosack

  • Prepare 0.23.0 #2654 [maxnoe] no-changelog-needed documentation-only

    • Tobychev

      • Tobychev: Suggest changing to "Fix that ensures"

      • Tobychev: Suggest changing to used by ctapipe.visualiation.CameraDisplay

created at 2024-11-19 06:30 by dinghy 1.3.3.