Skip to content

Do not get B field from external file anymore, but from CCDB - QualityControl#1692

Merged
Barthelemy merged 7 commits into
AliceO2Group:masterfrom
chiarazampolli:geomCCDB
Mar 29, 2023
Merged

Do not get B field from external file anymore, but from CCDB - QualityControl#1692
Barthelemy merged 7 commits into
AliceO2Group:masterfrom
chiarazampolli:geomCCDB

Conversation

@chiarazampolli
Copy link
Copy Markdown
Contributor

@chiarazampolli chiarazampolli changed the title Do not get B field from external file anymore, but from CCDB Do not get B field from external file anymore, but from CCDB - QualityControl Mar 28, 2023
@chiarazampolli chiarazampolli changed the title Do not get B field from external file anymore, but from CCDB - QualityControl [WIP]Do not get B field from external file anymore, but from CCDB - QualityControl Mar 28, 2023
@chiarazampolli chiarazampolli changed the title [WIP]Do not get B field from external file anymore, but from CCDB - QualityControl Do not get B field from external file anymore, but from CCDB - QualityControl Mar 28, 2023
@chiarazampolli
Copy link
Copy Markdown
Contributor Author

Hello @iravasen , @IsakovAD , @tomas-herman ,

The change is to allow to remove the use of external files for Geom and B field.
Can you please take a look at the PR also in O2DPG AliceO2Group/O2DPG#977?
For MFT, I think you don't need at all the geometry, not sure though.

Note that if these tasks are used at P2, you will need to update the json in consul as done for O2DPG as soon as a new tag will be deployed there.

Chiara

@tomas-herman
Copy link
Copy Markdown
Collaborator

Hi @chiarazampolli,

We need the geometry information because we are using a method to access additional information about the clusters: https://github.com/AliceO2Group/AliceO2/blob/346604524e24649cf599da2d0757759234bcaca1/Detectors/ITSMFT/MFT/alignment/src/AlignPointHelper.cxx#L206

And this helper needs the geometry information. As far as I understand the geometry needs to be available in the QC task for the helper to be able to work. I will comment in the O2DPG about the second PR.

Cheers,
Tomas

@chiarazampolli
Copy link
Copy Markdown
Contributor Author

Hello @tomas-herman ,
I tried and everything worked, because to my understanding, changing the json adding the GeomRequest makes the geometry available.
Can you please try with the PRs from O2, O2DPG, QC locally, and see if it behaves correctly? I checked a few MFT plots and before and after the change they were identical, but I did not check them all.
Chiara

@tomas-herman
Copy link
Copy Markdown
Collaborator

Hi @chiarazampolli

I will try to do a local test but I need to rebuild first as I have old versions of the repositories.

Just from looking at the code it seems to me that the Geometry Request in the json i.e.
"grpGeomRequest" : { "geomRequest": "Aligned", "askGRPECS": "false", "askGRPLHCIF": "false", "askGRPMagField": "true", "askMatLUT": "false", "askTime": "false", "askOnceAllButField": "false", "needPropagatorD": "false"

needs to be added here: https://github.com/AliceO2Group/QualityControl/blob/master/Modules/MFT/mft-clusters.json


mGeom = o2::its::GeometryTGeo::Instance();
mGeom->fillMatrixCache(o2::math_utils::bit2Mask(o2::math_utils::TransformType::L2G));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason to move these two lines in monitorData instead of initialize (where they were before)? The same for the ITSFhrTask.cxx

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point, that does not seem optimal

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The geometry is passed only when the task calls "run", which for the QC is the monitorData. Unfortunately, I don't see a better way. You can protect this: if already done, don't redo it. Assuming the geometry does not change.
I would let you take care once the PR is merged.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that with the DPL CCDB fetcher the geometry will not be loaded until the 1st TF processing. And the methods above to need TGeometry loaded.
As far as I understand, this means that they should be executed in the monitor.
Another question is that they should be executed only once in the beginning of the run, i.e. should be changed to

if (pc.services().get<o2::framework::TimingInfo>().globalRunNumberChanged) {
   mGeom = o2::its::GeometryTGeo::Instance();
   mGeom->fillMatrixCache(o2::math_utils::bit2Mask(o2::math_utils::TransformType::L2G));
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @shahor02 ,
Exactly, this is what I meant. I did not know about the way to check that the run number is changed, I would have checked with a bool :-)
Implemented now, please check.
For the B field, it is different though, since for that the DP processing is continuously ongoing.
Chiara

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @shahor02, @chiarazampolli.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chiarazampolli can you please apply the same also for the ITSClusterTask.cxx?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did actually, but I forgot to add it :-) Now done.

@iravasen
Copy link
Copy Markdown
Collaborator

@chiarazampolli thanks! I only have the comment above, the rest is fine! I will take care of modifying the ITS jsons at P2 once this will be there.

@chiarazampolli
Copy link
Copy Markdown
Contributor Author

Hi @chiarazampolli

I will try to do a local test but I need to rebuild first as I have old versions of the repositories.

Just from looking at the code it seems to me that the Geometry Request in the json i.e. "grpGeomRequest" : { "geomRequest": "Aligned", "askGRPECS": "false", "askGRPLHCIF": "false", "askGRPMagField": "true", "askMatLUT": "false", "askTime": "false", "askOnceAllButField": "false", "needPropagatorD": "false"

needs to be added here: https://github.com/AliceO2Group/QualityControl/blob/master/Modules/MFT/mft-clusters.json

Ciao @tomas-herman ,
Added.
Chiara

Copy link
Copy Markdown
Collaborator

@njacazio njacazio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Chiara!

@Barthelemy
Copy link
Copy Markdown
Collaborator

@tomas-herman do you sign off the changes ?

@Barthelemy
Copy link
Copy Markdown
Collaborator

merging as people have approved and only 1 check is missing

@Barthelemy Barthelemy merged commit 956d650 into AliceO2Group:master Mar 29, 2023
knopers8 pushed a commit to knopers8/QualityControl that referenced this pull request Apr 5, 2023
…yControl (AliceO2Group#1692)

* Do not get B field from external file anymore, but from CCDB

* also for TOF PID

* Also for ITS and MFT

* Also for TOF digits QC

* Request by Ivan

* Another json

* Update geometry only at the beginning of the run
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

6 participants