[Bug Report] Not Point 5 Compatible

bregnery commented 4 years ago

The Analysis tools are not compatible with the Point 5 chamber names.

Brief summary of issue

Any analysis of chambers at Point 5 currently fails as a result of the GE21 updates. Now, the software will not accept the Point 5 chamber names.

Here are the offending lines: https://github.com/cms-gem-daq-project/gem-plotting-tools/blob/76fa9ea7f86735c5ef23ff0f2305709a2b1009d4/utils/anautilities.py#L104-L111

Types of issue

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

Expected Behavior

Current Behavior

DAC scans for optohybrids in 0xc completed
====================
Analyzing VFAT3 DAC Scan Data
====================
Loading info from input TTree
An unexpected exception has occured: 'ge11m32l1'
ge11m32l1
DAC Scan Analysis Failed
Conncetivity Testing Failed
Goodbye

Steps to Reproduce (for bugs)

Analyze a DAC scan or run testConnectivity.py with a DAC scan for a P5 chamber

Possible Solution (for bugs)

Fix these lines: https://github.com/cms-gem-daq-project/gem-plotting-tools/blob/76fa9ea7f86735c5ef23ff0f2305709a2b1009d4/utils/anautilities.py#L104-L111

Context (for feature requests)

Your Environment

Version used:

Available Packages
Name        : gempython_gemplotting
Arch        : x86_64
Version     : 1.0.0
Release     : centos7.python2.7
Size        : 168 k
Repo        : cms_gem_cc7_64
Summary     : None
URL         : https://cms-gem-daq-project.github.io/gem-plotting-tools
License     : MIT

Shell used: Bash on point 5 computers

bregnery commented 4 years ago

These change was made by @dteague during #252

jsturdy commented 4 years ago

The issue is because until P5 we are associating detectors by their detector name, e.g., <STATION>-X-<SUBTYPE>-<PROD_SITE>-<ID>, e.g., GE11-X-S-INDIA-0016, GE21 preproduction is using GE21-M5-P5, GE2/1 production will use...? While at P5, the chambers are associated by their location: <CMS_STATION><ENDCAP><POSITION>L<LAYER> (and add -<SUBTYPE> only for compatibility with current lookup tables), e.g., GE11m34L1-L, GE21m34L1-M6, ME0m34L1 (GE21 and ME0 are guesses, but mostly irrelevant at present) As I see it, we have a couple of options: 1) Standardize the naming convention everywhere

Downside is that associating a chamber by "name" at P5 is not ideal 2) Standardize delimiters
Change CMS names to <STATION>-<ENDCAP>-<POSITION>L<LAYER>-<SUBTYPE> e.g., GE11-M-34L1-L 3) Make the tools better handle the options
Both situations we'll have <STATION><separator><other>, so parse based on the possible separators, ['m','p','-']

I think 2 is the most straightforward, and 3 would be fine too, but could be problematic in the parsing. However, 2 is problematic if the previously provided tools had already been using the detector name as a field in the output files, as they would have taken the detector name from the system_specific_constants, which now will need to be updated to the new convention... (@AndrewLevin do you know if this would have been the case?)

lpetre-ulb commented 4 years ago

Option 2 chosen and implemented for P5 operation with legacy software. Fixing naming parsing could be tricky in legacy release. Implementation expected to be different with xDAQ-based software (database powered).

lpetre-ulb commented 4 years ago

Since fixing this issue is a major refactoring that will likely result in breakages and since there is a workaround implemented for P5, it is probably better not trying to fix this issue.

cms-gem-daq-project / gem-plotting-tools