[Feature request] Allow manual override of the chipIDs in getCalInfoFromDB.py

[lpetre-ulb]

Brief summary of issue

While preparing the ULB setups with VFAT3 hybrid V2, retrieving and writing the proper IREF values and ADC and CAL_DAC calibration values revealed to be a long and error-prone task.

Since the Reed-Muller encoding is not implemented in hybrids V2 the getCalInfoFromDB.py is unfortunately useless.

This feature request aims at providing an option to manually override the chip IDs read through the VFAT3 slow controls. Therefore getting the nominal IREF values and ADC and CAL_DAC calibration values would be as simple as filling a list of chip IDs.

Types of issue

• Bug report (report an issue with the code)
• Feature request (request for change which adds functionality)

Expected Behavior

The user should have the possibility to manually override some chip IDs when using VFAT3s for which the Reed-Muller encoding is not used (or not working).

I propose to add a new option to getCalInfoFromDB.py which would point to a file containing the VFAT positions and chip IDs which must be overridden :

2 1234
5 8888
14 4242

If the option is set but the filename not provided then data are retrieved from a well-know location. For example a file named chipIDOverride.txt next to calFile_ADC0_${DET_NAME}.txt in the $DATA_PATH.

Current Behavior

Chip IDs are only read from the VFAT through slow controls and decoded from the Reed-Muller encoding.

Context (for feature requests)

VFAT3 hybrids v2 are used in test stands inside and outside of CERN. Standard tools should be usable.

Alternative proposal

The chip ID override file could be written somewhere on the CTP7. Whenever a chip ID is requested the CTP7 module would read the overrides from there. Since this alternative proposal requires to edit files on the CTP7 and that the file do not "follow" the detector I would prefer the first solution.

Your Environment

• Version used: dae6fb9

[bdorney]

Gonna go ahead and veto the alternative proposal immediately; we are not going to add any additional tedious text file interaction on/with the CTP7. This is definitely the wrong way to go.

Also I don't want to add another text file under $DATA_PATH/detectorName; this is really not the path we should pursue either.

Instead I would favor the implementing a subparser (similar to run_scans.py, sca.py, ana_scans.py, etc...) that would feature the options commands:

• readFromFile
• readFromDet

Each subcommand would set their own func argument and then after all parsing was done the main would call the dedicated function (e.g. args.func(<inputs>)).

Here the command readFromDet would preserve the current behavior. And readFromFile would have the following positional arguments:

• chamberName, the name of the detector/GEB pair to be used to store the output under $DATA_PATH.
• [inputVFATFile, listOfVFATs], mutually exclusive option group specifying file containing VFAT chip ID's (one per line) or comma separated list of chipID's to query

Then readFromFile would just need to call getGEMDBView or one of it's wrapper functions from dbutils.py.

However the functions in dbutils.py if supplied with a vfatList that is not None where all designed to join on vfatN and assumed the order of elements of vfatList followed the VFAT position. So a subsequent issue would be required to discuss how to change this and accommodate queries of less than 24 VFATs.

[bdorney]

Tagging @mexanick as relevant for GE2/1, specifically this comments:

However the functions in dbutils.py if supplied with a vfatList that is not None where all designed to join on vfatN and assumed the order of elements of vfatList followed the VFAT position. So a subsequent issue would be required to discuss how to change this and accommodate queries of less than 24 VFATs.

[mexanick]

@bdorney I don't think that the size of vfat list would introduce any issues. dbutils.py doesn't have any hardcoded limits of this array, so it should be just fine.

[bdorney]

@bdorney I don't think that the size of vfat list would introduce any issues. dbutils.py doesn't have any hardcoded limits of this array, so it should be just fine.

The requirement on fixed array size comes from joinOnVFATSerNum(), see:

https://github.com/cms-gem-daq-project/gem-plotting-tools/blob/e8c3f02ff82b32b91b4ab35d439b9fe2eb42731c/utils/dbutils.py#L141-L147

[lpetre-ulb]

Gonna go ahead and veto the alternative proposal immediately; we are not going to add any additional tedious text file interaction on/with the CTP7. This is definitely the wrong way to go.

I fully agree with you; it was only for completeness.

Also I don't want to add another text file under $DATA_PATH/detectorName; this is really not the path we should pursue either.

Ok, I forget this idea. The users will have to carefully keep track of the files containing the chip IDs then. A comment though, I would consider this file rather important among all the text files under $DATA_PATH/detectorName since the non-encoded chip IDs cannot be retrieved from the database nor from the VFAT3 itself.

Instead I would favor the implementing a subparser (similar to run_scans.py, sca.py, ana_scans.py, etc...) that would feature the options commands:

* `readFromFile`
* `readFromDet`

Each subcommand would set their own func argument and then after all parsing was done the main would call the dedicated function (e.g. args.func(<inputs>)).

Here the command readFromDet would preserve the current behavior. And readFromFile would have the following positional arguments:

* `chamberName`, the name of the detector/GEB pair to be used to store the output under `$DATA_PATH`.
* [`inputVFATFile`, `listOfVFATs`], mutually exclusive option group specifying file containing VFAT chip ID's (one per line) or comma separated list of chipID's to query

Then readFromFile would just need to call getGEMDBView or one of it's wrapper functions from dbutils.py.

I'm not sure a sub-parser is the best way to go. It will create two nearly independent commands and is less versatile than an optional overwrite. For example how do you deal with a mixture of hybrids V2 and V3? It has been done this week and might be useful for GE2/1 tests.

What about systematically reading the chip IDs from the VFATs and override the specified VFATs, if requested. I cannot really see a drawback with this option which is fully versatile.

However the functions in dbutils.py if supplied with a vfatList that is not None where all designed to join on vfatN and assumed the order of elements of vfatList followed the VFAT position. So a subsequent issue would be required to discuss how to change this and accommodate queries of less than 24 VFATs.

I think the changes can be quite straightforward if the vfatList is continuous; that can be part of the PR.

[bdorney]

Ok, I forget this idea. The users will have to carefully keep track of the files containing the chip IDs then. A comment though, I would consider this file rather important among all the text files under $DATA_PATH/detectorName since the non-encoded chip IDs cannot be retrieved from the database nor from the VFAT3 itself.

The VFAT3 has a barcode on it and the ID can be read from there. The sysadmin of a setup can choose to keep this file wherever they like; I just don't see having code supporting this file in a defined location (e.g. $DATA_PATH/detectorName) as necessary.

For example how do you deal with a mixture of hybrids V2 and V3? It has been done this week and might be useful for GE2/1 tests.

The use case I outlined would completely cover this scenario; either via a file input or the list input.

What about systematically reading the chip IDs from the VFATs and override the specified VFATs, if requested. I cannot really see a drawback with this option which is fully versatile.

In the end we are still perpetuating a $DATA_PATH like structure here rather than having a "confDB" for P5/QC8/local-test-stand. I would prefer development moved in this direction and I think so would @jsturdy.

I think the changes can be quite straightforward if the vfatList is continuous; that can be part of the PR.

Another idea I had would be to change this to vfatMap and then rely on hw_constants.py to specify the max VFAT list to loop over.

[bdorney]

But what's the long-term goal here? In principle we've "Frozen" the legacy branches for some time now but we are continuing to push to them (things come up of course).

Given that the current python tools exist for you to query a specific set of VFATs a la:

from gempython.gemplotting.utils.dbutils import getVFAT3CalInfo
myCalInfo = getVFAT3CalInfo( [ 0x17d4 + vfat for vfat in range(24) ] )

What's our strategy? Invest here or invest in the "future tool?"

I agree having functionality here that doesn't lock us in to a chipID read is good; just not sure where to put the best effort.

Any comments from @mexanick or @jsturdy ?

[lpetre-ulb]

For example how do you deal with a mixture of hybrids V2 and V3? It has been done this week and might be useful for GE2/1 tests.

The use case I outlined would completely cover this scenario; either via a file input or the list input.

I'm not sure how the V3 hybrid chip IDs can be recovered automatically with your method? My proposal is very simple for the user; you would only need to run a command like :

getCalInfoFromDB.py 1 4 0x1 --override [yourChipIDFile]

where yourChipIDFile contains only a subset of the VFATs.

In the end we are still perpetuating a $DATA_PATH like structure here rather than having a "confDB" for P5/QC8/local-test-stand. I would prefer development moved in this direction and I think so would @jsturdy.

I agree that this information should be retrievable from the DB, even a local XML DB. However I'm afraid that the software is not ready yet.

Given that the current python tools exist for you to query a specific set of VFATs a la:

from gempython.gemplotting.utils.dbutils import getVFAT3CalInfo
myCalInfo = getVFAT3CalInfo( [ 0x17d4 + vfat for vfat in range(24) ] )

What's our strategy? Invest here or invest in the "future tool?"

Sure this solution can work, but convenient is it? Remember that we will soon be running 6 test stands with v2 hybrids at ULB. Any user (Gilles, maybe a new person and myself) should be able to exchange a VFAT and update all the config files.

What's our strategy? Invest here or invest in the "future tool?"

I agree having functionality here that doesn't lock us in to a chipID read is good; just not sure where to put the best effort.

This is a very quick development which is really important for regular operation during GEB&OH QC. And as you rightfully stated multiple times the tools we use at ULB should be mainlined. Truth is that I could have completed the feature with the time I spent writing this issue. ;-)

[bdorney]

I'm not sure how the V3 hybrid chip IDs can be recovered automatically with your method? My proposal is very simple for the user; you would only need to run a command like :

getCalInfoFromDB.py 1 4 0x1 --override [yourChipIDFile]

where yourChipIDFile contains only a subset of the VFATs.

In your use case here I really don't see how this is different from what I outlined with the subparsers.

[lpetre-ulb]

I'm not sure how the V3 hybrid chip IDs can be recovered automatically with your method? My proposal is very simple for the user; you would only need to run a command like :

getCalInfoFromDB.py 1 4 0x1 --override [yourChipIDFile]

where yourChipIDFile contains only a subset of the VFATs.

In your use case here I really don't see how this is different from what I outlined with the subparsers.

I guess I must not understand well how your readFromFile sub-parser should work. Does it only change the calibration values of the specified VFATs and leave the others untouched?

In my use case yourChipIDFile can contain only a few VFAT chip IDs to be overridden; all the others chip IDs are read through the slow controls. For instance if you only want to manually override the chip IDs of VFATs 3, 6 and 9, you can use the following file :

3 1111
6 9999
9 1212

[bdorney]

I'm not sure how the V3 hybrid chip IDs can be recovered automatically with your method? My proposal is very simple for the user; you would only need to run a command like :

getCalInfoFromDB.py 1 4 0x1 --override [yourChipIDFile]

where yourChipIDFile contains only a subset of the VFATs.

In your use case here I really don't see how this is different from what I outlined with the subparsers.

I guess I must not understand well how your readFromFile sub-parser should work. Does it only change the calibration values of the specified VFATs and leave the others untouched?

In my use case yourChipIDFile can contain only a few VFAT chip IDs to be overridden; all the others chip IDs are read through the slow controls. For instance if you only want to manually override the chip IDs of VFATs 3, 6 and 9, you can use the following file :

3 1111
6 9999
9 1212

I still think this is the wrong way to go. To have this file in the correct location you'd need to have the mapping from geographic address to chamber name, this will increase reliance on chamber_config dictionary.

I think for all the reasons I've mentioned so far we should really just have the subparser that I've described. This type of operation should not really be done often; e.g. once you have the IREF's on the VFAT3 config files on the CTP7 (can be done with this tool) and the calibration files to disk there's no need for further DB queries in the case of "old hybrids" without the E-Fuse issue in the chipID.

[lpetre-ulb]

I guess I must not understand well how your readFromFile sub-parser should work. Does it only change the calibration values of the specified VFATs and leave the others untouched?
In my use case yourChipIDFile can contain only a few VFAT chip IDs to be overridden; all the others chip IDs are read through the slow controls. For instance if you only want to manually override the chip IDs of VFATs 3, 6 and 9, you can use the following file :

3 1111
6 9999
9 1212

I still think this is the wrong way to go. To have this file in the correct location you'd need to have the mapping from geographic address to chamber name, this will increase reliance on chamber_config dictionary.

I really don't understand what is the difference regarding file management between your sub-parser and my proposal. Since your first reply the idea of a default chip ID stored at a well known location is ruled out.

In any case the chip ID file must be written somewhere, but none of our proposals require the chip ID to be written at a specific location. As you said, this is the sysadmin job.

[bdorney]

Let's discuss this at the SW meeting tomorrow to get other developer's feedback since it seems we both disagree; the best way I think to proceed is to get the community's input.

Could you prepare a single slide with the "tag line" of the two proposals and show example use cases of each?

[bdorney]

Let's discuss this at the SW meeting tomorrow to get other developer's feedback since it seems we both disagree; the best way I think to proceed is to get the community's input.

Could you prepare a single slide with the "tag line" of the two proposals and show example use cases of each?

Of course I'll go with majority opinion ;)

[lpetre-ulb]

Let's discuss this at the SW meeting tomorrow to get other developer's feedback since it seems we both disagree; the best way I think to proceed is to get the community's input.
Could you prepare a single slide with the "tag line" of the two proposals and show example use cases of each?

Yes, sure. I might not be fair though since I think there is some misunderstanding on my side. So please correct me. ;-)

[bdorney]

One final comment is that adding a text file for a single setup/link is not such a big deal; but imaging the larger scale test stands. @mexanick is running the GE2/1 stand and is presently working with 4 links; this will eventually move to 8 I suspect. The Coffin is running one link but will expand to two. QC7 is running 2 links that change constantly. And QC8 is running up to 30 links that change with regular frequency.

So a sysadmin in this model has to maintain 30+ files across multiple locations, and the number of files will change as detectors move through QC7/8. Whereas with the subparser command you can pass one file at runtime for the specific use case; e.g. in your case those files must always exist. Whereas in the sub parser case there only needs to be an input file if that input command is requested.

[bdorney]

Okay now I'm done until tomorrow ;)

[lpetre-ulb]

Here is a summary of what was decided during this morning SW developer meeting.

TLDR; The two proposal will be implemented in order to provide all the required features and the maximal versatility.

The getCalInfoFromDB.py interface will be modified to provide two sub-parsers :

• readFromCmd: reads a list of VFAT chip IDs from the command line
• readFromDet: preserves the currently functionality and add the override option

readFromCmd only takes a list of VFAT chip IDs for which the user wants to read the calibration data. The results from the DB query are displayed on screen and can be saved under the $ELOG_PATH if requested with the --write2File option.

readFromDet preserves the current behavior. It however adds the possibility to provide an override file. If the option is provided the chip ID of chosen VFAT positions is overridden with a user defined value. This option enables easy operation for VFAT3 hybrid V2 setups and VFAT3 hybrid V2-V3 mixed setups.

[jsturdy]

One followup that I didn't stress enough (explicitly) during the meeting is that if this feature is targeting a "get it done now" release branch, we should prefer non API changes (@lpetre-ulb's initial proposal), and postpone the API changes for the redesign.
If this feature is already targeting the "future way things are done", then this comment is moot.
This comes to a head as the recent API changes in cms-gem-daq-project/gem-plotting-tools#225 resulted in a challenging rebase for cms-gem-daq-project/gem-plotting-tools#231, which, fortunately, testing of the code after the rebase did its job and caught.