Algorithm for LAr ByteStream reading
For @damazio and @pavol attention: A possible implementation of Re-entrant algorithm reading LArRawChannels for ByteStream.
Notes/Comments:
- So far reading the entire RawEvent and using the eformat::helper::create_toc to filter out LAr-fragments. Do we have somewhere a list of all LAr ROB-ids?
- I did not implement all the checks for data corruption we have in the old converter
- In this simple version, the cxx is less than 150 lines long with only ~30 lines of boilerplate code that could be shared with conversion-algorithms of LAr(Calib)Digits or LArFEBHeader.
Comments are welcome!
Merge request reports
Activity
As discussed with @pavol yesterday, extended this prototype to read also Digits and the data from the headers. They are stored in the same ROD-Fragment, so it makes sense to decode them at the same time. The implementation is still less than 300 lines and I believe it's well structured. That compares to >1000 lines in the reading part of old converter that we wrote in 2003 (and extended since then).
The actual decoding of the bits & bytes written by the readout electronics is done by the same RodBlockStructure classes that are used by the old converter I did not dare to touch them.
@damazio : If you stick to the service-approach for data-reading in the HLT, I think the RodBlockStructure classes are the only piece of code worth being shared between HLT and offline. The boilerplate code around is only some 10 lines.
Hello Walter,
I think I will need a bit more time to check all this. That said, I think we should not decode the digits with the energies. This can slow down things. Can we keep the LArRodDecoder instead?! I will try to read everything in the next two or three days. Please, remember that I also have to deal with the Tile, which has a similar interface (fillCollectionsHLT) which I would prefer to keep (see that I DID NOT read yet the code..) Thanks, Denis
-
Hi @damazio,
the decoding of RawChannels / Digits / FebHeaders is optional. One can turn each of them off by jobO.
For the trigger code, do you actually need the LArRawChannel object? I thought you are bypassing this step. I am pretty sure you'll need only a handful of code-lines from the LArRodDecoder (which is an overloaded and clumsy class)
- Walter
This merge request affects 5 packages:
- Calorimeter/CaloRec
- Event/ByteStreamCnvSvc
- LArCalorimeter/LArCnv/LArByteStream
- LArCalorimeter/LArROD
- Reconstruction/RecExample/RecExCommon
Adding @pavol as watcher
added Calorimeter EDM JetEtmiss LAr Reconstruction master review-pending-level-1 labels
added 1 commit
- 89dfb6ed - LArRawDataReadingAlg: Reseve vectors before filling
-
This merge request affects 5 packages:
- Calorimeter/CaloRec
- Event/ByteStreamCnvSvc
- LArCalorimeter/LArCnv/LArByteStream
- LArCalorimeter/LArROD
- Reconstruction/RecExample/RecExCommon
Adding @pavol as watcher
-
-
Athena AthSimulation externals 
cmake make required tests 
optional tests Full details available at NICOS MR-25404-2019-08-07-17-56
-
Athena AthSimulation externals cmake make required tests optional tests Full details available at NICOS MR-25404-2019-08-07-18-10
Prb another instance of https://its.cern.ch/jira/browse/ATR-20147 today
Edited by Christos Anastopoulos-
I assume !25436 (merged) fixes the issue. Jenkins, please retry a build
-
This merge request affects 5 packages:
- Calorimeter/CaloRec
- Event/ByteStreamCnvSvc
- LArCalorimeter/LArCnv/LArByteStream
- LArCalorimeter/LArROD
- Reconstruction/RecExample/RecExCommon
Adding @pavol as watcher
-
Athena AthSimulation externals cmake N/A N/A make N/A N/A required tests N/A N/A optional tests N/A N/A 
-
This merge request affects 5 packages:
- Calorimeter/CaloRec
- Event/ByteStreamCnvSvc
- LArCalorimeter/LArCnv/LArByteStream
- LArCalorimeter/LArROD
- Reconstruction/RecExample/RecExCommon
Adding @pavol as watcher
-
Athena AthSimulation externals cmake make required tests optional tests Full details available at NICOS MR-25404-2019-08-08-04-50
-
Hello Walter,
I can see that only a few types are instantiated (not all the different calibration types, transparent types, etc..). Won’t this be a future headache?! All the others have been phased out or left to old releases. I would like to do something similar in the HLTCalo actually.. We could reduce the size of the LArRodDecoder itself.
The execute method is like a large fillCollection. I fear it would be great to keep it in a LArRodDecoder, if possible unifying everything. Otherwise, code may change in the HLT and not be followed in the offline and vice-versa.. Maybe we could do that instead? What do you think? The tool would be controlled by this algorithm. Would you mind if I do that?!
I am worried about the need of some particular conditions (cabling, first of the FEBs in a ROD payload, etc). Maybe we could by pass that in a check in the first event?!
Do we have a simple running jobOption that I could play with this?! I would like to check the processing time of this thing.
As for the different types and converters, I think you are doing the right thing on removing all these stupid converters. We could control it to use the different types by options as you did, of course.
What about encoding?! I think only HLTCalo in RDO->BS mode worries about it (and transient BS).
-
Hi Denis,
at this point, I worry only about the data types used in standard reconstruction. For the LAr(Accumulated)CalibDigitContainer I plan to write two more algorithms like this one (not urgent). That would be another ~ 300 lines of code.
As I wrote earlier, there is so little boilerplate code that it's not worth trying to share it in a dedicated tool like LArRodDecoder. Moreover, the LArRodDecoder has specialized
fillCollectionmethods for each data type. So there is actually no common code, it's just collected in one tool instead of two or three algorithms. The only thing worth sharing is the LArRodBlockStructure. Apart of that, there is really nothing that could diverge between offline and HLT.There are (new-style) jobO in the python directory. My tests yesteday indicate that the decoding time is about 4ms/event. Vtunes suggests the bulk of that is indeed the the new'ing of LArRawChannels.
Writing bytestream is a different story. The MR here does not address this.
- Walter
-
would you mind if I try after this?! Can you tell me which jobOption. Also, I need a fast course on how to deal with this new configuration. You probably know of many. Could I bug you to point one out to me?! I think we can do a bit better than this (even though, this is great). I do believe people will soon complain about the other types of data.. Cheers, Denis
-
To run the new jobO (via a pickle file)
python LArRawDataReadingConfig.pyathena LArRawDataReading.pklThe only missing data types are the ones used for the electronic calibration. We'll should move this code to new-style athena as well but - as I said already - this is not a priority.
- Walter
Edited by Walter Lampl added review-pending-level-2 label and removed review-pending-level-1 label
mentioned in merge request !25730 (merged)
-
added review-user-action-required label and removed review-pending-level-2 label
added 1 commit
- f7a1acaa - Implement review comments (copyright, ...) + fix flake8 complain
-
This merge request affects 5 packages:
- Calorimeter/CaloRec
- Event/ByteStreamCnvSvc
- LArCalorimeter/LArCnv/LArByteStream
- LArCalorimeter/LArROD
- Reconstruction/RecExample/RecExCommon
Adding @pavol as watcher
added review-pending-level-1 label and removed review-user-action-required label
-
Athena AthSimulation externals cmake make required tests optional tests Full details available at NICOS MR-25404-2019-08-10-17-17
This is a bit weird, there is so much DEBUG output here: http://atlas-computing.web.cern.ch/atlas-computing/links/distDirectory/ci/CIWebArea/nicos_web_areaMRCIbuilds64BC7G8AthenaOpt/NICOS_TestLog_MR-25404-2019-08-10-17-17/ReleaseTests___Trigger_MT_required-test__Trigger_MT_required-test__m.html and it ultimately fails with Athena CRITICAL stopped by user interrupt terminate called without an active exception I wonder if this was turned on by mistake? Tagging @hartj and @jpanduro in case they know of anything or have seen this elsewhere?