Add Calibration.from_data()

[philsmt]

One of the original ideas behind the CalibrationData API is to integrate the determination of detector condition from data, i.e. the various properties of control devices and/or the data itself. Due to various changes over the years to hardware and/or software, this can unfortunately be somewhat messy and is nowadays encoded in calibration notebooks, amongst other places.

This is an attempt to bring this functionality to the CalibrationData API in EXtra. Ideally, it makes obtaining calibration data as simply as:

run = open_run(...)
cd = CalibrationData.from_data(run)

As it happens, the new design lends itself to this quite nicely by separating the detector-specific and -agnostic parts as before across the CalibrationData class and ConditionsBase implementation:

• Each detector-specific ConditionsBase object gains a .from_data() class method taking an extra_data.DataCollection as well as the CalCat detector name. From that, it attempts to initialize itself by inferring the necessary fields from the data. For AGIPDConditions e.g., its signature is:

  @classmethod
  def from_data(cls, data, detector_name, modules=None,
                fpga_comp=None, mpod=None, fpga_control=None, xtdf=None,
                client=None, **params):

  fpga_comp, mpod, fpga_control, xtdf are inferred from the detector entry in CalCat and what's in data, or can be overwritten manually. The same applies to the modules list, which allows taking only the given modules into account (e.g. when a module is powered down, but part of the data).

  **params can be any of the regular parameter conditions, and prevents it from being inferred from data. The object attempts to be as greedy as possible and determine it from any applicable source, e.g. acquisition rate can be taken from the AGIPD FPGA comp device or the difference of pulse IDs in the data stream. If the sources in data are insufficient to construct the condition, an exception is thrown. This can be prevented by passing the missing parameters manually.

  For diagnostics, the actual extraction of any given parameter from data is implemented in its own static method, but that is an optional pattern.

• The CalibrationData class itself gains a .from_data() class method taking an extra_data.DataCollection and optionally a detector name. From that, it attempts to infer the detector name if missing via EXtra-data components, its detector type via CalCat and this pick the corresponding ConditionsBase type. Also, it infers begin_at from the data and thus attempt to initialize a proper .CalibrationData object via .from_condition. Any additional keyword arguments are passed to the conditions object, e.g. to overwrite parameters or sources.

For now, I implemented the necessary code to infer the condition for AGIPD, LPD and Jungfrau based on what we use in pycalibration, including (most) of its idiosyncracies. For AGIPD, I isolated the exact pycalibration code and ran it against this new API for a sample run of every proposal an AGIPD correction ever ran on with full agreement on all of them.

You can find a minimal example showcasing several situations here.

[takluyver]

Not remotely important to the overall PR, but this can be one line with the builtin any() function.

[philsmt]

Certainly, but explicitly checking for is None seemed to become fairly clunky: any([x is None for x in [a, b, c]])

As I was already writing a function for it, the allocation cost for the comprehension seemed unnecessary.

[takluyver]

Do we have a use case in mind, or is this a general 'don't throw away information'?

If we don't have a concrete use case, I might be inclined to get rid of it, or make it ._condition, just for debugging. Making it easy to assume constants always come with a condition object could make it harder to use the flexibility of finding constants in other ways - like from a report in CalCat, from the correction metadata of a run, or combining different groups of constants.

It should also be possible to look up the conditions associated with each individual constant in CalCat, at least where we have the IDs, which is a more general option.

[philsmt]

The idea was to expose the entire condition inferred from data, i.e. access the generated ConditionsBase object. I didn't consider looking into individual CCVs for their condition. Technically one could ConditionsBase.from_data() before to obtain it, but that means leaving out on inferring detector type and creation date from data.

[takluyver]

Our other error messages generally start with a capital letter (although admittedly this isn't as general a convention as I thought it was - Python itself seems to prefer lowercase)

[philsmt]

The latter is indeed the reason I generally try to stick with lowercase, but you're right EXtra-data mostly uses upper cases with EXtra being somewhere in the middle.

Would you prefer to set a general rule?

[philsmt]

Rebased on top of DetectorData, old HEAD was 9914ee67428c63d9d0f2219e4a5164c18f2ee38e

[codecov]

Codecov Report

❌ Patch coverage is 65.26316% with 99 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.74%. Comparing base (6c1c1b5) to head (feef5da).

Files with missing lines	Patch %	Lines
src/extra/calibration.py	65.26%	99 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #373      +/-   ##
==========================================
- Coverage   73.14%   72.74%   -0.40%     
==========================================
  Files          35       35              
  Lines        6248     6521     +273     
==========================================
+ Hits         4570     4744     +174     
- Misses       1678     1777      +99     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[philsmt]

Rebased on top of the main branch after merging DetectorData, now to add some tests.

[philsmt]

I've started adding tests replicating the ones done in pycalibration for operating condition detection from data. This has presented complications:

• The mock data framework always uses the current time as train timestamps, which ends up in the CalCat query and thus breaks the cassettes.
• All relevant parts of the mock data code is in EXtra-data, which makes adjusting it to cover more test cases difficult.
• EXtra-data itself does not actually use that mock data for its tests

We have encountered the latter issue already in pycalibration. The proper solution to me would be to move all that mock data code to EXtra, including the mock devices only used in those. I would prefer to not grow this MR even more however, so I'd like to leave it with these tests for now and then move stuff over.