Project

General

Profile

Actions

Development #2411

closed

wrong Mvd position after the origin shift

Added by Sergey Gorbunov 8 months ago. Updated 7 months ago.

Status:
Closed
Priority:
Urgent
Assignee:
Oleg Golosov
Target version:
Start date:
01/31/2022
Due date:
% Done:

100%

Estimated time:
Spent time:

Description

Hi Eoin,

After the change of the origin (commit e6c87d7aea2b8ac46fa97bc2f7242ec0912b6947 ),
Mvd stations got the wrong Z positions.
I see the following problems:

1. All STS stations are shifted by -40 cm, while MVD stations are shifted by -44 cm.

2. There are almost no MVD points produced (see the plots)

3. The Z positions of MVD stations obtained from CbmMvdDetector are biased by +2 cm from the MC points.
CbmMvdDetector::Instance()->GetParameterFile()->GetZPosition(Station) returns this:

[INFO] L1: Mvd station 0 at z -34
[INFO] L1: Mvd station 1 at z -30
[INFO] L1: Mvd station 2 at z -26
[INFO] L1: Mvd station 3 at z -22

Florian, I think it is important to fix it before the release.

PS. It is weird that the commit passed the CI test. In my simulation, the L1 tracker calls an assertion already on the second event.

Here are the distributions:

MvdPoints before the shift (100 mbias sis100 events):

MvdPoints after the shift:

Sts points before the shift:

Sts point after the shift:

Cheers,
Sergey


Files

MvdPointZBeforeShift.png (74.9 KB) MvdPointZBeforeShift.png Sergey Gorbunov, 01/31/2022 06:24 PM
MvdPointZAfterShift.png (87.6 KB) MvdPointZAfterShift.png Sergey Gorbunov, 01/31/2022 06:24 PM
StsPointZBeforeShift.png (108 KB) StsPointZBeforeShift.png Sergey Gorbunov, 01/31/2022 06:25 PM
StsPointZAfterShift.png (91.2 KB) StsPointZAfterShift.png Sergey Gorbunov, 01/31/2022 06:25 PM
mvd_v20e.png (37.5 KB) mvd_v20e.png Geometry with mvd_v20e_tr Florian Uhlig, 02/16/2022 04:45 PM
mvd_v20c.png (80.8 KB) mvd_v20c.png Geometry with mvd_v20c_tr Florian Uhlig, 02/16/2022 04:45 PM
MVDPoints_mvd_v20a_tr.pdf (14 KB) MVDPoints_mvd_v20a_tr.pdf Z-position of MVDPoints with mvd_v20a_tr Florian Uhlig, 02/16/2022 05:16 PM
Actions #1

Updated by Eoin Clerkin 8 months ago

  • Status changed from New to Rejected
  • Target version set to DEC21

I am aware of this issue, but it is not exactly a bug per se. It can be solved easily by generating a new binary but this binary would only be intermediately used.

Let me explain, the target is due to move to -44cm from the centre of the magnet, yet the current test setups are to be validated off the APR21 geometries which have the target at -40cm.

The MVD_v20d is a shifted version of MVD_v20c which is the MVD version to be used when the target move is realized, i.e. DEC21 geometries, which is where we want to get to.

Solution to this issue can be solved by generating a MVD geometry with the old target position but is it necessary? I will temporarily reject this issue but please reopen it if is necessary to have this intermediate MVD geometry and I will then generate it.

Actions #2

Updated by Sergey Gorbunov 8 months ago

Hi Eoin,

If I got it right - it is ok that Mvd is shifted a bit more than Sts.

Ok, but what is with the other two problems: why there are only few MC points in MVD and in the first Sts station?
It looks like in my simulations the target is still somewhere at Z=0. I'm using standard run_transport.C.
Should I set the new target position manually? We need to modify this macro then.

Also, the third problem stays: In the new geometry, CbmMvdDetector returns biased Z positions of Mvd stations. (It is not a question to your, but it has to be fixed.)

Sergey.

Actions #3

Updated by Sergey Gorbunov 8 months ago

Hi Eoin,

I think I know what is the problem. In run_transport.C we set the target properties:

run.SetTarget("Gold", 0.025, 2.5);

but this SetTarget() also sets XYZ to the default values:

void SetTarget(const char* medium, Double_t thickness, Double_t diameter, Double_t x = 0., Double_t y = 0.,
Double_t z = 0., Double_t rot = 0.);

Actions #4

Updated by Eoin Clerkin 8 months ago

Hi Sergey,

My understanding is that we have moved away from the run_transport.C macro. Instead we are using run_tra_file.C and run_tra_beam.C. If you look into these you'll see the following

  // -----   Target properties   --------------------------------------------
  // For flexibility, the target volume is not part of the predefined
  // geometry setup. It thus has to be specified in this macro. By default,
  // a Gold target of 250 micrometer thickness and 2.5 cm diameter is used.
  // The target shifts by 4 cm upstream by decision of the technical board
  // in Apr 2020 which is 40 cm from center of magnet.
  const char* targetMedium = "Gold";
  Double_t targetThickness = 0.025;  // in cm
  Double_t targetDiameter  = 2.5;    // in cm

  Double_t targetZpos = -40.0;
  // The target position is at z=0 for the old coordinate system but is intended
  // to be moved to -4cm in the DEC21 release. The global coordinate system will
  // also shift from the old target position to the center of the magnet which
  // is a net displacement of -40 cm. In terms of the new coordainte system
  // the target is therefore to be at -44 cm. In order not to cause forgetting
  // we will automate the shifting process for a short time, until the full move
  // has been completed.
  if (strstr(setup, "_APR21")) targetZpos = 0.0;
  if (strstr(setup, "_DEC21")) targetZpos = -44.0;
  std::cout << "Target is at " << targetZpos << "cm from origin" << std::endl;

So it automatically sets the target position depending on the setup choice. Are you having the same issue when you use run_tra_file.C ?

Actions #5

Updated by Sergey Gorbunov 8 months ago

Eoin,

Thanks for the hint! I was still using run_transport.C. With run_tra_file.C the target is in the right position.
We should probably delete run_transport.C to avoid confusion.

Sergey.

Actions #6

Updated by Sergey Gorbunov 8 months ago

Eoin,

Could you please make "sis100_electron" setup be exactly equal to "sis100_electron_Apr21", but shifted to -40cm?
I think it is needed. I expect a lot of debugging and this geometry would be a very useful tool to analyze the effect of the origin shift.

As it is now, it is had to judge whether the reconstruction results differ due to the origin shift or due to the relative shifts between the detector parts.

Cheers,
Sergey.

Actions #7

Updated by Eoin Clerkin 8 months ago

  • Tracker changed from Bug to Feature

@sergey Could you change the status of this issue from rejected to open? I don't seem to be able to do this.

Actions #8

Updated by Eoin Clerkin 8 months ago

  • Tracker changed from Feature to Development
Actions #9

Updated by Eoin Clerkin 8 months ago

  • Project changed from CbmRoot to Framework
Actions #11

Updated by Eoin Clerkin 8 months ago

  • % Done changed from 0 to 90
Actions #12

Updated by Florian Uhlig 8 months ago

  • Status changed from Rejected to In Progress
Actions #13

Updated by Oleg Golosov 8 months ago

I tried to run the simulations using standard run_tra_file.C with setup sis100_electron and MVD v21e and cannot see a single MVD point.
Seems that the target is located after MVD.

FairRoot v18.2.1, FairSoft jun19p2

Actions #14

Updated by Florian Uhlig 8 months ago

Hi Oleg,

I think this is easy to understand when you check the geometry. For the attached plots of the geometry I did two simulation of the sis100_electron setup, one with the current mvd geometry v20c_tr and one with v20e_tr.

As you can see in the plots for v20e_tr the mvd is in front of the targetbox and the target. So it is no wonder why there are no MCPoints.

Actions #15

Updated by Florian Uhlig 8 months ago

Okay, I think meanwhile I understand the problem. The MVD is the only detector which is not placed inside the top volume but inside a volume which is inside the pipe. Shifting the pipe by 40 cm also shifts the mvd by 40 cm. Eoin now shifted also the the mvd in mvd_v20e_tr.root by 40 cm which results in a total shift of 80 cm.

In my opinion the correct mvd file to be used in sis100_electron setup is mvd_v20a_tr.root. When using this mvd geometry I have MVDPoints at -32, -28,-24 and -22 cm which, as I understand the discussion, are the correct values.

@Oleg, @Sergey,

please try with mvd_v20a_tr and check if this solves your problems.

The second issue of the return positions of the stations I will check after I got confirmation that the MCPoints are correct.

Actions #16

Updated by Oleg Golosov 8 months ago

Thanks, Florian, v20a seems to suit our needs.

Actions #17

Updated by Florian Uhlig 8 months ago

Hi Oleg,

could you please also check if the third problem is gone

3. The Z positions of MVD stations obtained from CbmMvdDetector are biased by +2 cm from the MC points.
CbmMvdDetector::Instance()->GetParameterFile()->GetZPosition(Station) returns this:

Actions #18

Updated by Eoin Clerkin 8 months ago

Although I was already aware of this, thanks Florian Uhlig for clarifying this to Oleg.

This MVD geometry is not suitable to be substituted into the default setups as they currently stand, and this was the original reason for rejecting the request for this geometry. As said in meeting, this geometry is to be used when there was a redefinition of the pipevac1 volume in the newer beampipes by Mehul. So for example, it could be used with beampipe_v21a and sts_v21a. I have not tested this, but it should be the case.

To restate, it is a mistake to use this new MVD geometry with the v16 beampipes used in sis100_electron_APR21 and sis100_electron setups.

Actions #19

Updated by Eoin Clerkin 8 months ago

@o.golosov If v20a suits, can this issue be closed?

Is there no longer a need for mvd_v21e, can the MR be closed without merging? With merging?

Actions #20

Updated by Florian Uhlig 8 months ago

Eoin Clerkin wrote in #note-19:

@o.golosov If v20a suits, can this issue be closed?

We should wait for the discussion in the software meeting, change the setups, create the merge request and add it to the master as well as the dec21_patches branch.

Actions #21

Updated by Oleg Golosov 8 months ago

Florian Uhlig wrote in #note-17:

Hi Oleg,

could you please also check if the third problem is gone

3. The Z positions of MVD stations obtained from CbmMvdDetector are biased by +2 cm from the MC points.
CbmMvdDetector::Instance()->GetParameterFile()->GetZPosition(Station) returns this:

the positions are correct now

Actions #22

Updated by Florian Uhlig 7 months ago

@Eoin Clerkin,

was the setup updated? I don't see a change in the geometry repository.

Actions #23

Updated by Florian Uhlig 7 months ago

Eoin Clerkin,

was the setup updated? I don't see a change in the geometry repository.

Florian Uhlig wrote in #note-22:

@Eoin Clerkin,

was the setup updated? I don't see a change in the geometry repository.

Actions #24

Updated by Florian Uhlig 7 months ago

  • Status changed from In Progress to Resolved
  • Assignee changed from Eoin Clerkin to Oleg Golosov

@o.golosov

the fixes are now in the release branch dec21_patches. Please redo your tests with that branch and either close the issue if the problem is gone or report otherwise.

Actions #25

Updated by Oleg Golosov 7 months ago

Solved for both master and dec21_patches.
I do not have permission to close the issue.

Actions #26

Updated by Florian Uhlig 7 months ago

  • Status changed from Resolved to Closed
  • % Done changed from 90 to 100

Thanks for confirmation. I will close the issue.

Actions

Also available in: Atom PDF