Fix fail on reinjection of constants for dynamic flat-field correction
Description
This suppresses the reinjection exception for Offsets
and DynamicFF
constants because operators uses the same dark multiple times to generate constants.
How Has This Been Tested?
Hotfixed in production
Relevant Documents (optional)
Exception traceback
---------------------------------------------------------------------------
CCVAlreadyInjectedError Traceback (most recent call last)
Cell In[6], line 23
15 ccv_root = write_ccv(
16 tempf.name,
17 pdu['physical_name'], pdu['uuid'], pdu['detector_type']['name'],
18 constant_name, conditions, constant['creation_time'],
19 proposal, [dark_run, flat_run],
20 constant["data"], constant['dims'])
22 if db_output:
---> 23 inject_ccv(tempf.name, ccv_root, metadata_folder)
25 if local_output:
26 ofile = f"{out_folder}/const_{constant_name}_{db_module}.h5"
File ~/deployments/development/git.xfel.eu/detectors/pycalibration/pycalibration-2024-
↪→11-13-3.15.3-188b319/src/cal_tools/constants.py:283, in inject_ccv(const_src, ccv_
↪→root, report_to)
278 # TODO: Remove this when the new injection code is added.
279 if (
280 resp['status_code'] == 422 and
281 "taken" in resp['app_info'].get("begin_at", [""])[0]
282 ):
--> 283 raise CCVAlreadyInjectedError
284 else:
285 raise RuntimeError(resp)
CCVAlreadyInjectedError:
Types of changes
- Bug fix (non-breaking change which fixes an issue)
Reviewers
Merge request reports
Activity
This is fine for a hotfix - I've cherry-picked d30a01a1 - but I don't quite follow the description of why it's wanted. Do people generate only some constants the first time, and then run it again generating a superset? Are they tweaking parameters to improve the constants? And if so, do they expect that the last constants generated will win, instead of the first?
For merging to master, I think we should at least have some message printed so you can tell from the output whether each constant was actually injected or not.
The notebook computes two set of constants:
Offset
andDynamicFF
. The first is just the processing of dark run. The second is PCA decomposition of flat-field run, but the offsets applied to compute PCA.The notebook computes them in one go and for this it takes numbers of dark and flat-fields runs on input.
At the same time offsets are really not changing much over time but the taking of the dark run takes a quite time.
As results users want to use the same dark run for a few different flat-fields. The second processing of dark gives the same result as at first time, so there is no reason to inject, but also doesn't matter if you notify or not - the constants are in the DB
Initially exception was suppressed only for
Offsets
, but then for third reason the processing was indicated as failed in myMDC, but constants were really injected. It forced users to request processing once again. And then it failed on reinjection ofDynamicFF
constants. At the end all reinjection exception were suppressed.The underling library actually prints out the notification about reinjection
Edited by Egor SobolevGot it, thanks!
the processing was indicated as failed in myMDC, but constants were really injected. It forced users to request processing once again. And then it failed on reinjection of
DynamicFF
constants.I'd argue that it should fail in that scenario, unless we verify that the colliding constant in the database is equivalent to the one we've failed to inject. So I might suppress the exception only for offsets, and say the extra bug to fix is whatever caused the processing to fail after it had already injected constants. I don't feel strongly about that, though.
LGTM
You're correct, Thomas. I ended up ignoring both as some mysterious problem caused a dark run that completed successfully (based on SLURM jobs and slurm.out) to be marked as FAILED in myMdC. It was likely a one-off thing now, but for operational convenience I made it ignore both.
I want to post the details of said run later to investigate.
changed milestone to %3.15.5
mentioned in commit ee5329b7