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

MFT: new MC track task #1468

Merged
merged 4 commits into from
Mar 8, 2024
Merged

Conversation

diana0x0f
Copy link
Contributor

No description provided.

@diana0x0f
Copy link
Contributor Author

Hi @chiarazampolli, I have added the new MFT MC task. Could you please check if everything is correct? Thanks!

@benedikt-voelkel
Copy link
Contributor

@diana0x0f we will also see if the QC goes through since there is a small test that will automatically take your new QCs into account.
The CI will start at some point.

@diana0x0f
Copy link
Contributor Author

Hi @benedikt-voelkel ,
I tested the workflow locally without any problem, all the new QC objects were created. How can we reproduce the problem that occurred in the CI test? Thanks!

@benedikt-voelkel
Copy link
Contributor

Hej @diana0x0f ,
the easiest would be under the following assumptions:

  • your local development directory of O2DPG is located at $O2DPG_DEV.
  • your working directory is clean (everything committed, nothing staged, no changed files)
  • your development is in the commits on top, maybe it consists even of only one commit. But let's assume the last ${N} commits are the ones that contain you development.

Then run

O2DPG_TEST_REPO_DIR=${O2DPG_DEV} O2DPG_TEST_HASH_BASE=HEAD~${N} ${O2DPG_ROOT}/test/run_workflow_tests.sh

That should run the same thing.

Let me know if you experience any issues.
More details can be found here: https://aliceo2group.github.io/simulation/docs/generators/generatorconfig.html#run-the-test-locally
(note that this specifically refers to run_generator_tests.sh, however you need run_workflow_tests.sh as stated above)

@benedikt-voelkel
Copy link
Contributor

This needs AliceO2Group/QualityControl#2115 which does not seem to be contained in any QC tag.
@Barthelemy are there any plans to include it and also to bump QC to a new tag in alidist which would then have the necessary developments?

@Barthelemy
Copy link
Contributor

@benedikt-voelkel There will be a tag tomorrow. Is it ok to have a new tag or do you need a patch on an existing QC tag ?

@benedikt-voelkel
Copy link
Contributor

I guess it would be important to have that one in before we merge this.
From my side, a new QC tag would be sufficient. In that case we will wait until you bump it in alidist.
Pinging also @chiarazampolli @JianLIUhep to understand how urgent this is.

@chiarazampolli
Copy link
Collaborator

If this is for MC anchored to data, we need to patch the QC tag that is used for MC for those data.

@Barthelemy
Copy link
Contributor

Ok, so I am going to tag today a new version and if you want me to patch a specific tag, just let me know. It is very quick to do (perhaps ping me on MM).

@chiarazampolli
Copy link
Collaborator

Hello @diana0x0f ,
For which production would you like this PR to be included?
Chiara

@diana0x0f
Copy link
Contributor Author

Hi @chiarazampolli , this new MC task was not targeted specifically for any particular production at the moment. Our intention was to incorporate it into the codebase for any upcoming central MC production.

@benedikt-voelkel
Copy link
Contributor

@chiarazampolli I guess no labels needed, right?
From my side, good to be merged.

@benedikt-voelkel
Copy link
Contributor

@TimoWilken
I think the CI was successful on this already a few days ago, maybe I am mistaken. Now it is failing due to fetching RCT from CCDB.

@TimoWilken
Copy link
Contributor

Hi @benedikt-voelkel, sorry, known issue: https://its.cern.ch/jira/browse/O2-4735. The next automatic rebuild should fix it. Let me know if it's urgent, then I'll restart it manually, or you can merge it right away.

@benedikt-voelkel
Copy link
Contributor

Thanks!
Let's wait for the rebuild, then, I will merge.

@benedikt-voelkel benedikt-voelkel merged commit 9943c5b into AliceO2Group:master Mar 8, 2024
6 checks passed
@chiarazampolli
Copy link
Collaborator

chiarazampolli commented Mar 13, 2024

Hello @benedikt-voelkel ,
Sorry for coming back late to this: if this is needed for PbPb apass3 and MC (MC will be done with the tag for data, this is why I mention apass3 data even if the PR is for MC), then labels should be added.
Cheers,
Chiara

@benedikt-voelkel
Copy link
Contributor

I understood that it is not urgent but as @diana0x0f said

Hi @chiarazampolli , this new MC task was not targeted specifically for any particular production at the moment. Our intention was to incorporate it into the codebase for any upcoming central MC production.

No rush from my side.
But of course, it could be that @diana0x0f means to include it if possible before starting "any" new MCs. Could you comment?

Personally, I cannot comment.

@diana0x0f
Copy link
Contributor Author

Hi @benedikt-voelkel , @chiarazampolli ,
we would like to have this included for the PbPb apass3 and MC that Chiara mentioned.
What I meant with the previous comment was indeed to have it included for "any" new MCs.

@chiarazampolli
Copy link
Collaborator

Ciao @diana0x0f ,

I added your task here:

https://gitlab.cern.ch/bvolkel/o2dpgdocs/-/blob/main/docs/software/requests/2023pbpb_apass.md

Please, do like that next time you need a cherry-pick in a production tag.

Chiara

@chiarazampolli
Copy link
Collaborator

BTW, do we need also to update the QC? Otherwise your task might not work. If this is the case, we need to ask for a tag to @Barthelemy .

@diana0x0f
Copy link
Contributor Author

Hi @chiarazampolli , thanks! This task in QC was included in v1.135.0.

@chiarazampolli
Copy link
Collaborator

Hello @diana0x0f ,

Then we need to retag QC. @Barthelemy , could you do so? We currently use v1.126.5 in production.

Note the PR by @lmassacr too: she might need an update as well for QC (#1531), and the one by @martenole (#1467). They also might need an update of QC.

Chiara

@JianLIUhep
Copy link
Contributor

Hi @chiarazampolli, @Barthelemy,
I think the PR for MFT in QC was already retagged for v1.126.6, which also includes the fix for HMPID. Then, another patch to HMPID was released with v1.126.7.

So, if I am not mistaken, now we might only need to retag QC to include QC updates for #1531 and #1467 if that's the case.

@noferini noferini removed the reco label Apr 18, 2024
noferini pushed a commit that referenced this pull request Apr 18, 2024
noferini pushed a commit that referenced this pull request Apr 19, 2024
noferini added a commit that referenced this pull request Apr 23, 2024
noferini added a commit that referenced this pull request Apr 23, 2024
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.

7 participants