Bugfix in tof placement
It is discovered in issue2446 that the tof is placed incorrectely in some setups. This fixes the issue. Corrects placement in both v21a and v20c geometries.
Merge request reports
Activity
added 3 commits
-
3e3df161...8124d461 - 2 commits from branch
computing:master - 65537800 - Bugfix in tof placement
-
3e3df161...8124d461 - 2 commits from branch
assigned to @v.friese
requested review from @n.herrmann
For the tof_v20c Z position used in sis100_electron setup, current versus suggested update is shown
Edited by Eoin Clerkinadded 3 commits
-
0ec2e561...8124d461 - 2 commits from branch
computing:master - 65397612 - Bugfix in tof placement
-
0ec2e561...8124d461 - 2 commits from branch
tof_v21a_1e which is default in electron DEC21 is moved by 2cm. It should be noted that the change of coordinate system to center of magnet is 40cm whilst the shift downstream due to space provision of the pipe is roughly 40cm. This gives the false distance equivalents between the _DEC21 and _APR21 setups.
Edited by Eoin Clerkinadded 3 commits
-
0a97e131...d079fc3d - 2 commits from branch
computing:master - 80ec6cd5 - Bugfix in tof placement
-
0a97e131...d079fc3d - 2 commits from branch
added 2 commits
added 5 commits
-
76243ec6...518a5a1b - 4 commits from branch
computing:master - cbc41bef - Bugfix in tof placement
-
76243ec6...518a5a1b - 4 commits from branch
The binaries for this MR has been merged to the geometry repo after approval of @i.deppner
CbmSoft/cbmroot_geometry!152 (merged)
If there are no comments or further suggestions, this MR may be merged IHMO.
Dear @f.uhlig, @n.herrmann, @i.deppner,
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
the problem I see with this MR is that it also changes quite a number of setups and changes the psd versions in these setups.
Did we agree that the new psd positions are okay and should be added in the setups?
@f.uhlig You are welcome to proposed a change to the setup files, such that the psd is not in its parking position for the hadron setup. Personally I have no objections, but I think I should continue and implement the current clearly articulated decision from the SWM 3 weeks ago and change it back, if we decide that, later. Additionally other psd geometries are to come after I am given the technically-verified beam pipe geometries which i currently wait on. Let me know if you'd like a short meeting to clarify this.
Edited by Eoin ClerkinI am fine with the decision to add the psd geometries and also change them in the setups. I am not fine with the fact that this happens behind the scenes with this commit. With this commit you want to change the TOF geometries which is also in the commit message but you also change invisibly the psd geometries and the setups.
Please create a separate merge request which only updates the hash for the geometry repository to
34f302c071bda80da0bb8d562937b66a31bc27cd
to bring in the psd changes with a clear commit message. I will take care to merge that one first and then I will this one.
@f.uhlig It is should be said that there is no attempt to conceal anything, this commit is only to include the tof as described, which requires the updating of the hash.
The change of the hash, to bring in the psd in its parking position, was merged earlier today 84094f36, which had been pending for about 2 weeks. I had the hash change in this commit but I removed it on Tuesday to prevent a potential merge conflict as there were already several intervening hash updates.
There is a problem here whereby a hash update may bring several geometries and changes in setup files of CBM and mCBM at the same time. I will make another merge request to update the hash to the current version.
could you please rebase locally.
added 16 commits
-
65526306...aee14091 - 15 commits from branch
computing:master - 51494f3f - Bugfix in tof placement
-
65526306...aee14091 - 15 commits from branch
added 2 commits
added 10 commits
-
614cd6b3...70194e26 - 9 commits from branch
computing:master - 1b7b8ef8 - Bugfix in tof placement
-
614cd6b3...70194e26 - 9 commits from branch
mentioned in merge request !767 (merged)
added 5 commits
-
1b7b8ef8...e335e610 - 4 commits from branch
computing:master - 46ea0572 - Bugfix in tof placement
-
1b7b8ef8...e335e610 - 4 commits from branch
As far as I can see, this MR is now focused following the merging of other MRs and brings only the single commit from the geometry repo.
Can we merge this when the pipeline finishes? or are we waiting for further review/decision?It is right now blocking !767 (merged)
- Resolved by Eoin Clerkin
in principle it is fine to merge. Unfortunately there is no approval from the TOF group. I can't judge if the change is okay or not.
- Last reply by Eoin Clerkin
deleted
Edited by Eoin Clerkinchanged milestone to %DEC21
added Ported label
