[AGIPD][CORRECT] Split FF and PC queries

Description

This is related to this ticket https://redmine.xfel.eu/issues/160639

Since using calcat_interface with AGIPD I was using SplitConditionCalibrationData to retrieve both PC and FF constants. PC constants are dark and FF constants are illuminated.

As the mentioned runs in the ticket were running in High CDS, there were no PC constants for the conditions but there were available FF. However, it was report that no FF nor PC constants were found because the whole function failed trying to query PC.

This fix is splitting the query for PC and FF to avoid such errors. The same logic is hotfixed in production at the moment.

How Has This Been Tested?

Relevant Documents (optional)

Types of changes

• Bug fix (non-breaking change which fixes an issue)

Checklist:

Reviewers

@schmidtp @kluyvert

assigned to @ahmedk

Hello @manettim,

We have a question related to the issue in this MR. We are currently querying AGIPD constants in 3 different processes. This is related to the different conditions of the 3 classes of constants. How bad is this influencing the performance? compared to querying everything in one shot as was initially planned.

Hi @ahmedk.

In general, for the get_by_detector_conditions API, there is a certain number of queries we need to send to the database which are fixed, independent from the number of constant types asked (aka "calibrations" in CalCat DB) such as: getting the PDUs for the given detector, getting the parameters by name, filtering the conditions. All of those queries are always sent to the database, and only once per API request, either if you ask for one constant type or all.

So, in principle, querying for all constant types in one go should be overall quicker than the total time needed to get the CCVs querying each constant type separately, but clearly the single request to get all will be longer. However, the internal caching of the database may play an important role, so that results for "recent" queries are available sooner than others, making the difference between N * Time_for_one_constant and 1 * Time_for_all_constants not that big, if you execute the N requests one after the other.

I am currently testing and keeping track of how many queries we need to satisfy the requests and how long it takes on the database side and at the moment my impression is that there is not much difference (for the total DB time).

However in your question you refer to different conditions: if you have different conditions, you should not be able to get all CCVs with a single API call in any case. Am I missing something?

After digging into the slowness issue, I found that the common queries, so to say, account at most for just 5% of the total database time. The bottleneck is in the CCVs retrieval, where we are actually missing a database index which could be a game changer. Still testing...

Conclusion: the fix on indexes which will be deployed next week should bring a dramatic improvement in the performances. I expect at least 5 times faster completion of the requests to get_by_detector_conditions.

As discussed, optimizing the API to account for small differences in the conditions for different constant types is sensible, but the improvement in the performance would be negligible, as the filters on the conditions filter are not, at the moment, the critical part to satisfy the request.

I would rethink of it after seeing the improvements due to the index optimization.

resolved all threads

added 1 commit

• 6a8e39dd - spelling mistake

Compare with previous version

Other than that, the change LGTM, and it's a useful thing to keep in mind for designing the new yet another CalCat API (!885 (merged)).

yes, exactly.

Thanks for linking the MR because I failed to remember if this was tracked in an issue or MR.

added 1 commit

• 724ab601 - intialize pc_constants and ff_constants lists

Compare with previous version

resolved all threads

merged

mentioned in commit 0ce8e50e

changed milestone to %3.11.2

This returns a set. dark_constants, pc_constants are lists. In line 649 they are concatenated, that raises exception

TypeError                                 Traceback (most recent call last)
<ipython-input-19-32ef92fb1091> in <module>
     15 modules.drop(mod)
     16
---> 17 warn_missing_constants = set(dark_constants + pc_constants + ff_constants)
     18 warn_missing_constants -= error_missing_constants
     19 warn_missing_constants -= set(calibrations)
TypeError: can only concatenate list (not "set") to list

Thank you @esobolev for reporting this! I will fix this. This didn't fail in the tests because x-ray gain is not used in the reference test. Which is something I am changing !902 (merged)

mentioned in merge request !909 (merged)