Add digi_sector files for MUCH mcbm v22j and v22k
Changes required in CbmMuchDigitizeGem and CbmMuchFindHitsGem Class to use this file. tX = 8.5 and tY = 81.0 for GEM and tX = -66.0 and tY = 72.0 for RPC.
Merge request reports
Activity
Thanks @a.agarwal_AT_vecc.gov.in.
In coming MRs, we will adapt your mentioned changes. For long run we need to find a robust scheme to implement such alignment measurements, probably in the geometry root file. Update if you have some ideas.
Vikas
The stuck/failing job is due to some missing permissions, I asked Florian to run his script to set them in a new infrastructure issue.
Whenever I get the email that it is done I will restart the last job.
Dear @v.singhal,
you have been identified as code owner of at least one file which was changed with this merge request.
Please check the changes and approve them or request changes.
added CodeOwners label
Yes, The file can be merged.
@a.agarwal_AT_vecc.gov.in after this kindly generate sector file for mMuCh v22k geometry also.
Thanks, Vikas
If I am not mistaken, the changes to the
v22k
will also need the changes toCbmMuchDigitizeGem
andCbmMuchFindHitsGem
to work, is that right?If yes, I will merge when we have both files ready, either in this MR or in this one and another one for
v22k
.
This way we can directly bring the hash update together with the changes thetX
andtY
in those two classes through a single "all matching" MR.As a side note: the places requiring changes are ~l. 948 for CbmMuchDigitizeGem and ~l. 611 for CbmMuchFindHitsGem, right?
Do you want to just replace the values used for mCBM 2019, or should a new control flag be introduced?Dear Pierre,
Yes, you are right for any new mMuCh geometry, in which position changes, then X and Y need to be changed (now 2 values, one for GEM and another for RPC). We are thinking to make this automatic using Geometry but till now we could not find the proper changes. If you have some ideas for the same then kindly guide us.
Presently our motivation is to make hit reconstruction available for High Rate mMuCh 2022 data. In this respect, modifications in related files for cbmroot project are ready. I'll generate a MR. In that MuchFindHitsGem changes are there.
For v22k, making sector file is possible, however, as you pointed out, with this, changes are required in CbmMuchDigitizwGem and CbmMuchFindHitsGem. So for time being kindly merge v22j. After this we will try for proper solution and merge accordingly.
Thanks and Regards Vikas
assigned to @p.-a.loizeau
added 4 commits
-
500a2452...1bfd83de - 3 commits from branch
CbmSoft:master
- c5d55823 - Changes required in CbmMuchDigitizeGem and CbmMuchFindHitsGem Class to use...
-
500a2452...1bfd83de - 3 commits from branch
@a.agarwal_AT_vecc.gov.in @v.singhal
As computing/cbmroot!1232 (merged) now hopefully brings compatibility with both
v22j
andv22k
, could one of you please add the digi_sector file forv22k
to this MR? (if simpler for you, sending me the file should also work as I can add commits here with the assignee role)I would afterward take care of merging it, adding a commit with the hash update to computing/cbmroot!1232 (merged) and merging it (should be doable without needing actions from you)
added 2 commits
@a.agarwal_AT_vecc.gov.in @v.singhal
I added a second commit (also with Apar as author) with the
v22k
file he sent me by email. I also reformated the message to make it clear what change is done here and what needs to be done on the Cbmroot side for each of them (for 22k I read the values from the "cxx Change" files also in Apar email).I compared to the values used by Vikas in computing/cbmroot!1232 (merged) and saw there are some differences especially for the RPC in
22k
.
Could you please have a quick cross check between these commit messages and the diffs and let me know which of the values match best?
(I suspect Apar's as he did the geometry, but just to be sure).In the meantime I will prepare an additional commit to Vikas MR to add the matching change to the
DigitizeGem
class as what he did in theFindHits
one, as it escaped our attention before (did not thought about simulation I must confess )mentioned in merge request computing/cbmroot!1232 (merged)