Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New grpGeomRequest for ITS and MFT jsons #1522

Merged
merged 2 commits into from
Mar 26, 2024

Conversation

iravasen
Copy link
Contributor

@iravasen iravasen commented Mar 5, 2024

This follows AliceO2Group/QualityControl#2164
and has to go together with it!

Before merge wait approval from MFT colleagues.

Copy link

github-actions bot commented Mar 5, 2024

REQUEST FOR PRODUCTION RELEASES:
To request your PR to be included in production software, please add the corresponding labels called "async-" to your PR. Add the labels directly (if you have the permissions) or add a comment of the form (note that labels are separated by a ",")

+async-label <label1>, <label2>, !<label3> ...

This will add <label1> and <label2> and removes <label3>.

The following labels are available
async-2022-pp-apass4
async-2023-pbpb-apass
async-2023-pp-apass1
async-data
async-mc
async-2022-pp-apass6

Copy link
Collaborator

@shahor02 shahor02 left a comment

Choose a reason for hiding this comment

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

Thanks @iravasen
Looks fine to me but better MFT experts also check. I've just noticed, that in opposite to the original QualityControl/Modules/MFT/mft-tracks.json the O2DPG/Data/production/qc-async/mft.json was not requesting the GRPGeom block for the MFTTracks task. This means that the call https://github.com/AliceO2Group/QualityControl/blob/55da459a99b14b920255998b1c40148dce48603f/Modules/MFT/src/QcMFTTrackTask.cxx#L211 was returning default 128 orbits / TF

@chiarazampolli
Copy link
Collaborator

Hello @iravasen ,
Did you ping the MFT expert? I guess it is Maurice, but I cannot find his github account.

Chiara

@mcoquet642
Copy link
Collaborator

Hi, after discussing with the MFT QC team these changes are ok from our side. Do you also plan to modify the config files in consul or should we do it ourselves once we have the new FLPSuite ?

@shahor02
Copy link
Collaborator

shahor02 commented Mar 6, 2024

Thanks @iravasen , @mcoquet642 !
Yes, eventually also the consul should be modified but only once the new QC is bumped and deployed at P2.
Also, this PR fails since it is built against QualityControl/v1.135.1-local2, I guess we should have it on hold until the QC is bumped (pending to @Barthelemy )

@iravasen
Copy link
Contributor Author

Hi @Barthelemy @shahor02, I guess now this PR can be merged?

@shahor02
Copy link
Collaborator

Yes! Once the new QC is deployed at P2, one can change also jsons on consul.

@chiarazampolli
Copy link
Collaborator

Hello @iravasen ,

I guess I can merge now. And I also think that this is needed in view of the memory reduction for QC, right?

Chiara

@iravasen
Copy link
Contributor Author

Yes thanks! Sorry

@chiarazampolli
Copy link
Collaborator

Now there are conflicts...

@chiarazampolli chiarazampolli merged commit fae72c3 into AliceO2Group:master Mar 26, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants