-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Revision of the PSimHit type storage, revert trackId to its basic meaning #49969
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 4 commits
3c0da1a
3659e0d
38d3de7
5606fe1
06141fb
b9528e6
0bfe599
74e840e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| #ifndef SimHitCategory_h | ||
| #define SimHitCategory_h | ||
|
|
||
| // 7 bits field availablen PSimHit processType, i.e. up 127 | ||
| static constexpr unsigned int k_procidMask_ = 0x1FF; | ||
| static constexpr unsigned int k_hitidMask_ = 0x7F; | ||
| static constexpr unsigned int k_hitidShift_ = 9; | ||
|
|
||
| namespace SimHitCategory { | ||
|
|
||
| // Identification code of sim hit production mechanism, subdetector dependent | ||
|
|
||
| static constexpr unsigned int k_BTLsecondary = 1; // BTL hit from secondary particle not saved in history | ||
| static constexpr unsigned int k_BTLlooper = 2; // BTL hit from identified looper | ||
| static constexpr unsigned int k_BTLfromCalo = 3; // BTL hit from back-scattering from CALO volume | ||
| static constexpr unsigned int k_ETLfromBack = 4; // ETL hit entering from a rear face of disks | ||
|
|
||
| }; // namespace SimHitCategory | ||
fabiocos marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| #endif | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,17 @@ | ||
| <lcgdict> | ||
| <class name="PSimHit" ClassVersion="10"> | ||
| <class name="PSimHit" ClassVersion="11"> | ||
| <version ClassVersion="11" checksum="1181355938"/> | ||
| <version ClassVersion="10" checksum="1181355938"/> | ||
| <ioread sourceClass = "PSimHit" version="[-10]" targetClass="PSimHit" source="unsigned int theTrackId" target="theTrackId"> | ||
| <![CDATA[ | ||
| if ((newObj->detUnitId() >> 25) == 0x31) {theTrackId = onfile.theTrackId % 200000000;} | ||
| ]]> | ||
| </ioread> | ||
| <ioread sourceClass = "PSimHit" version="[-10]" targetClass="PSimHit" source="unsigned int theTrackId; unsigned short theProcessType" target="theProcessType"> | ||
| <![CDATA[ | ||
| if ((newObj->detUnitId() >> 25) == 0x31) {theProcessType |= (onfile.theTrackId / 200000000) << 9 ;} | ||
| ]]> | ||
| </ioread> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @makortel sorry, do you perhaps manage to see what is wrong with this syntax? I took inspiration from other examples found in CMSSW, but the conversion of older format does not work, just simply running the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pcanal @vepadulano Any thoughts what could be going wrong here? The rules seem to have an expectation of upper one to be run before the lower one, but on the surface the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @makortel Where does the order dependency comes into play?
There is no 'real' promise on the ordering of rules executions. They will be executed after their input have been read into the
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Can you run the failing example under
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The upper rule updates
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see the issue with ordering, anyway I thought that the "onfile." prefix was ensuring to always look at the original input, not at its modified version. And in any case, even if the modified one is taken, that would result in missing the correct hit type, setting it to zero, but should not cause crashes. I also tried for the second rule the modified syntax: just to avoid mixing things, but it keeps crashing. The valgrind output is quite long, I can provide it if useful, the piece just before crashing looks like As far as I can see, the target must be for a single item, cannot share two of them, at least I tried and got an error message at compilation stage. So I should try a way through a conversion function. Is there a syntax to start fir a copy of the whole BTW, I modified the name of one, not strictly needed in principle, as I was not getting otherwise a different checksum to be used in the class version history.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The failing stack trace is odd and indicates some probable internal error and seems to be related to the fact that some collection is involved. I will need a reproducer with a debug build of ROOT to look into this further.
There can be (in principle) more than one target data member, although it usually makes more sense to only have one.
Not really but it should also not be (in principle) any more helpful that the current syntax.
I do not know about the CMS version checker but for ROOT proper, you can increase the version number explicitly without any change in the checksum.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
IIRC we have an earlier case where the version number was increased while keeping the checksum the same. |
||
| </class> | ||
| <class name="std::vector<PSimHit>"/> | ||
| <class name="edm::Wrapper<std::vector<PSimHit> >" splitLevel="0"/> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| <use name="SimDataFormats/TrackingHit"/> | ||
|
|
||
| <bin file="testPSimHit.cc"> | ||
| </bin> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| #include "SimDataFormats/TrackingHit/interface/PSimHit.h" | ||
|
|
||
| #include <iostream> | ||
| #include <iomanip> | ||
| #include <cassert> | ||
|
|
||
| int main() { | ||
| Local3DPoint dummy(0, 0, 0); | ||
|
|
||
| for (int procId = 1; procId < 404; procId++) { | ||
| for (unsigned short itype = 0; itype < 8; itype++) { | ||
| PSimHit testHit(dummy, dummy, 0., 0., 0., 11, 0, 0, 0., 0., procId); | ||
| testHit.setHitProdType(itype); | ||
| std::cout << " hit procId = " << std::fixed << std::setw(8) << procId << " sim procId = " << std::setw(8) | ||
| << testHit.processType() << " hit type = " << std::setw(8) << itype << " sim type = " << std::setw(8) | ||
| << testHit.hitProdType() << std::endl; | ||
|
|
||
| assert(procId == testHit.processType()); | ||
| assert(itype == testHit.hitProdType()); | ||
| } | ||
| } | ||
|
|
||
| return 0; | ||
| } |
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.