Implementation of the ITS Half Barrels #6584

mario6829 · 2021-07-05T14:49:22Z

The Half Barrels, an intermediate level of volumes between the Layer and the Stave, has been implemented in the ITS geometry tree. Now each Layer is composed by two Half Barrels, each one containing half of the Staves previously placed directly into the Layer volume. This structure is more realistic with respect to the real world and allow for a better misalignment of all involved elements. Anyway it should be noted that the Staves and all their subvolumes do not change their global position in the world reference system: simply they are now daughters of the Half Barrel volume which in turn is a daughter of the Layer volume.
New methods in the GeometryTGeo class were introduced to handle this new volume level, while where needed the existing methods in the GeometryTGeo and Detector classes were adapted to take into account this new level in the volume tree. The construction of the Layer geometry in the V3Layer class was changed too to actually implement the Half Barrel volumes.
In this implementation only the Staves and their own services are part of the Half Barrels. Work is in progress to include in the new volumes also those parts of the supporting structures which belong to the Half Barrels in the physical ITS.

mario6829 · 2021-07-05T14:51:15Z

Hi @shahor02 @iouribelikov here it is the PR of the Half Barrels. Let me know in case of problems or if you need more info. Kind regards

shahor02 · 2021-07-05T19:31:02Z

Hi @davidrohr @TimoWilken

The fullCI is amazingly green, but the log shows quite some warnings and errors:

Error while processing /O2/GPU/GPUTracking/TRDTracking/GPUTRDTracker.cxx.
/sw/SOURCES/O2/6584-slc8_x86-64/0/GPU/GPUTracking/TRDTracking/GPUTRDTracker.cxx:37:10: error: 'omp.h' file not found [clang-diagnostic-error]
#include <omp.h>
....
1 error generated.
Error while processing /sw/BUILD/de9f0d7f4b545fb32732038aff81f6daba397c25/O2/GPU/GPUTracking/Base/cuda/GPUReconstructionCUDArtc.src.S.
/sw/BUILD/de9f0d7f4b545fb32732038aff81f6daba397c25/O2/GPU/GPUTracking/Base/cuda/GPUReconstructionCUDArtc.src.S:1:2: error: expected identifier or '(' [clang-diagnostic-error]
        .global _curtc_GPUReconstructionCUDArtc_cu_src
        ^
Found compiler error(s).
....
End of search list.
3039 warnings and 9 errors generated when compiling for host.
Error while processing /O2/Detectors/ITSMFT/ITS/tracking/cuda/src/ClusterLinesGPU.cu.
error: cannot find libdevice for sm_20. Provide path to different CUDA installation via --cuda-path, or pass -nocudalib to build without linking with libdevice. [clang-diagnostic-error]
error: unknown argument: '-fPIC -O2 ' [clang-diagnostic-error]
error: unknown argument: '-forward-unknown-to-host-compiler' [clang-diagnostic-error]
error: unsupported option '--allow-unsupported-compiler' [clang-diagnostic-error]
error: unsupported option '--expt-relaxed-constexpr' [clang-diagnostic-error]
error: unsupported option '--extended-lambda' [clang-diagnostic-error]
error: unsupported option '--ftz=true' [clang-diagnostic-error]
error: unsupported option '--generate-code=arch=compute_86,code=[compute_86,sm_86]' [clang-diagnostic-error]
/sw/SOURCES/O2/6584-slc8_x86-64/0/GPU/Common/GPUCommonRtypes.h:32:12: error: 'Rtypes.h' file not found [clang-diagnostic-error]
  #include "Rtypes.h"

The FST itself finishes w/o errors, but I am not sure if I should trust it given all these errrors.

shahor02

Hi @mario6829

Thanks a lot! Looks fine but I have 1 doubt: naively I thought the half-barrel is lower level alignable than the layer (i.e. if the half-barrel contains all half-layers). But I see that in your implementation each layer is made of half-barrels, and those of different layers are independent.
Is this really supposed to be so or this is just an "effective" implementation?

Then, I am not sure a layer as alignable volume makes sense. Aren't top and bottom halves independent? If so, then I would suppress the

AliceO2/Detectors/ITSMFT/ITS/simulation/src/Detector.cxx

Lines 1049 to 1051 in 18b078c

    
           if (!gGeoManager->SetAlignableEntry(sname.Data(), path.Data())) { 
        
             LOG(FATAL) << "Unable to set alignable entry ! " << sname << " : " << path; 
        
           }

Finally, I saw than during tests you've uploaded an ideal new alignment object to http://ccdb-test.cern.ch:8080/browse/ITS/Align. Since this creates a lot of error messages with current dev, I've masked it with empty object. If you play again with test ocdb, please clean it (before this PR is merged) by

root
.L $O2_ROOT/share/macro/UploadDummyAlignment.C+
UploadDummyAlignment("http://ccdb-test.cern.ch:8080",0,-1, o2::detectors::DetID::getMask("ITS"));

mario6829 · 2021-07-06T07:57:03Z

Many thanks @shahor02 for reviewing the code!
For what concerns the layers: I implemented this way because I thought (maybe wrongly) that each single half barrel could be misaligned with respect to the others (in principle they are solidly linked to the support structures, but there may be a slight difference wrt the ideal position - this may be more relevant for MB/OB than IB). Indeed the two halves are independent, but I imagined that the layer itself could be misaligned as a whole (something like the whole ITS volume). In short: I preferred to leave open all possible degrees of freedom. But if you think this freedom is redundant, I'll remove the three lines you mention. In fact, I can even remove the layer as a volume if you think it is now useless (not in this PR, of course, I have to do it carefully - we can discuss in a WG meeting).
For the alignment object in the CCDB: I really apologize! I didn't do it on purpose: I was trying to use a local file instead of the one on the db, I don't really know how I could have uploaded it... sorry for that.
Thanks again. Cheers

davidrohr · 2021-07-06T09:59:50Z

Hi @davidrohr @TimoWilken

The fullCI is amazingly green, but the log shows quite some warnings and errors:

The FST itself finishes w/o errors, but I am not sure if I should trust it given all these errrors.

@shahor02 : That is OK, the warnings seem to be during the codechecker. The codechecker always has a lot of false warnings / errors during compilation. Important is the

========== List of errors found ==========

at the end.

iouribelikov · 2021-07-06T10:00:30Z

For what concerns the layers: I implemented this way because I thought (maybe wrongly) that each single half barrel could be misaligned with respect to the others (in principle they are solidly linked to the support structures, but there may be a slight difference wrt the ideal position - this may be more relevant for MB/OB than IB). Indeed the two halves are independent, but I imagined that the layer itself could be misaligned as a whole (something like the whole ITS volume). In short: I preferred to leave open all possible degrees of freedom. But if you think this freedom is redundant, I'll remove the three lines you mention. In fact, I can even remove the layer as a volume if you think it is now useless (not in this PR, of course, I have to do it carefully - we can discuss in a WG meeting).

@mario6829 @shahor02 As far as I understand,
a) the ITS consists (mechanically) of 4 half-barrels: two IB half-barrels and two OB half-barrels.
b) Within each of these 4 half-barrels, the corresponding half-layers could potentially be mis-aligned.
c) Then, the whole IB is mechanically independent of the OB. So the whole IB could be mis-aligned wrt the whole OB.
For me, this means that any of the layers (taken as whole) is not really "mis-alignable" wrt other layers.

shahor02 · 2021-07-06T12:27:10Z

@mario6829 @iouribelikov OK, I would propose to suppress the whole layers as alignable DOF, because in this case I cannot apply a constraint on the movements of the half-layers of the same half-barrel (i.e. effectively consider e.g. MB half-barrel as an alignable).

mario6829 · 2021-07-06T12:58:25Z

Ok @shahor02 I removed the alignement of Layer as you suggested. Then we can discuss how to properly re-arrange the whole volume tree. Thanks again. Cheers

shahor02

Thanks @mario6829 , should be fine now, with the half-layer being next to the full ITS coarse DOF I should be able to group them in global alignment (i.e. to constrain half-layers to move together).

shahor02 · 2021-07-06T13:12:26Z

Detectors/ITSMFT/ITS/simulation/src/Detector.cxx

  TString wrpV =
    mWrapperLayerId[lr] != -1 ? Form("%s%d_1", GeometryTGeo::getITSWrapVolPattern(), mWrapperLayerId[lr]) : "";
  TString path = Form("%s/%s/%s%d_1", parent.Data(), wrpV.Data(), GeometryTGeo::getITSLayerPattern(), lr);
  TString sname = GeometryTGeo::composeSymNameLayer(lr);


This line should not be needed anymore

Hi @shahor02 it was just a reminder that the layer was alignable and now it isn't anymore. But ok, I can remove it. Do you agree to do it in the next PR ? Otherwise I trigger again all the tests... :)
Many thanks. Cheers

iouribelikov · 2021-07-10T17:00:27Z

Detectors/ITSMFT/ITS/base/include/ITSBase/GeometryTGeo.h

  int getNumberOfModules(int lay) const { return mNumberOfModules[lay]; }
  int getNumberOfHalfStaves(int lay) const { return mNumberOfHalfStaves[lay]; }
  int getNumberOfStaves(int lay) const { return mNumberOfStaves[lay]; }
+  int getNumberOfHalfBarrels(int lay) const { return mNumberOfHalfBarrels[lay]; }


@mario6829 Do we need this function and the corresponding std::vector ? The number of half-barrels is always 2, is not it ?

Hi @iouribelikov I just wrote it like the other ones (pretty like the number of layers, which is actually fixed too but it has a member and a getter), but yes, I can change it into a constant (or in a single-valued member instead of a vector). @shahor02 do you agree ?

Done: I changed it to a single-valued member, the same for all layers.

shahor02 · 2021-07-12T08:16:20Z

Hi, I was not merging this PR since I understood @iouribelikov wants to discuss it at tomorrow meeting.
Fine for removing the line later

This reverts commit 4f52866.

iouribelikov

@mario6829 For me, everything works fine now. Thank you very much, Mario !

* Implementation of the ITS Half Barrels * Fixing ITSMisaligner macro for new method names with Half Barrels * Do not set Layer as alignable volume * Fixed number of half barrels * Revert "Fixed number of half barrels" This reverts commit 4f52866. * Fixed number of half barrels * Correctly add half barrel number to hit generation and chip index number Co-authored-by: Mario Sitta <Mario.Sitta@cern.ch>

Implementation of the ITS Half Barrels

b596c07

mario6829 requested review from bovulpes, iouribelikov and rpezzi as code owners July 5, 2021 14:49

Fixing ITSMisaligner macro for new method names with Half Barrels

18b078c

shahor02 reviewed Jul 5, 2021

View reviewed changes

Do not set Layer as alignable volume

d5b8cc2

shahor02 reviewed Jul 6, 2021

View reviewed changes

iouribelikov reviewed Jul 10, 2021

View reviewed changes

Mario Sitta added 3 commits July 19, 2021 12:24

Fixed number of half barrels

4f52866

Revert "Fixed number of half barrels"

5ae3344

This reverts commit 4f52866.

Fixed number of half barrels

dde23f5

mario6829 force-pushed the itsgeo branch from b70a116 to dde23f5 Compare July 22, 2021 09:08

Correctly add half barrel number to hit generation and chip index number

1b90ea1

iouribelikov approved these changes Jul 22, 2021

View reviewed changes

shahor02 approved these changes Jul 27, 2021

View reviewed changes

shahor02 merged commit 329a45d into AliceO2Group:dev Jul 27, 2021

mario6829 deleted the itsgeo branch July 27, 2021 10:01

	if (!gGeoManager->SetAlignableEntry(sname.Data(), path.Data())) {
	LOG(FATAL) << "Unable to set alignable entry ! " << sname << " : " << path;
	}

Implementation of the ITS Half Barrels #6584

Implementation of the ITS Half Barrels #6584

Uh oh!

Conversation

mario6829 commented Jul 5, 2021

Uh oh!

mario6829 commented Jul 5, 2021

Uh oh!

shahor02 commented Jul 5, 2021

Uh oh!

shahor02 left a comment

Choose a reason for hiding this comment

Uh oh!

mario6829 commented Jul 6, 2021

Uh oh!

davidrohr commented Jul 6, 2021

Uh oh!

iouribelikov commented Jul 6, 2021

Uh oh!

shahor02 commented Jul 6, 2021

Uh oh!

mario6829 commented Jul 6, 2021

Uh oh!

shahor02 left a comment

Choose a reason for hiding this comment

Uh oh!

shahor02 Jul 6, 2021

Choose a reason for hiding this comment

Uh oh!

mario6829 Jul 12, 2021

Choose a reason for hiding this comment

Uh oh!

iouribelikov Jul 10, 2021

Choose a reason for hiding this comment

Uh oh!

mario6829 Jul 12, 2021

Choose a reason for hiding this comment

Uh oh!

shahor02 Jul 12, 2021

Choose a reason for hiding this comment

Uh oh!

mario6829 Jul 20, 2021

Choose a reason for hiding this comment

Uh oh!

shahor02 commented Jul 12, 2021

Uh oh!

iouribelikov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants