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.

Sign up for GitHub

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

Jump to bottom

Change to using Collisions = Collisions_001 #10478

Merged
merged 2 commits into from
Dec 18, 2022
Merged

Change to using Collisions = Collisions_001 #10478

merged 2 commits into from
Dec 18, 2022

Conversation

ddobrigk
Copy link
Contributor

@ddobrigk ddobrigk commented Dec 13, 2022 •
edited
Loading

  • this commit changes to the collision table with the correct CovYY and CovXZ elements, following the discussion in the monday meeting
  • All other changes are in place for this: the converter exists and has been merged 4 days ago in O2Physics
  • once this PR is merged, a general announcement will follow and all users must add the collision-converter workflow (with no parameters required) to use any current AO2D

@ddobrigk
Change to using Collisions = Collisions_001
9ba05e3 
* this commit changes to the collision table with the correct CovYY and CovXZ elements, following the discussion in the monday meeting [1] 
* All other changes are in place for this: the converter exists and has been merged 4 days ago in O2Physics [2]
* once this PR is merged, a general announcement will follow and all users must add the `collision-converter` workflow (with no parameters required) to use any current AO2D

[1] https://indico.cern.ch/event/1219192/contributions/5128952/attachments/2542060/4376840/DDChinellato-MondayMeeting-7Nov2022-1.pdf
[2] https://github.com/AliceO2Group/O2Physics/blob/master/Common/TableProducer/collisionConverter.cxx
@ddobrigk ddobrigk marked this pull request as ready for review December 13, 2022 14:05
@ddobrigk ddobrigk requested a review from a team as a code owner December 13, 2022 14:05
@ddobrigk
Copy link
Contributor Author

@AliceO2Group/framework-admins can I please get an "approve" on this? Then I will immediately notify everyone. Super thanks!

@shahor02
Copy link
Collaborator

Hi @ddobrigk

It breaks the

10/105 Test  #24: test_Framework_test_AnalysisTask ........................***Failed    0.66 sec
Running 2 test cases...
/System/Volumes/Data/build/alice-ci-workdir/o2/sw/SOURCES/O2/10478/0/Framework/Core/test/test_AnalysisTask.cxx:171: �[1;31;49merror: in "AdaptorCompilation": check task2.inputs[0].binding == "Collisions_000" has failed [Collisions_001 != Collisions_000]�[0;39;49m
/System/Volumes/Data/build/alice-ci-workdir/o2/sw/SOURCES/O2/10478/0/Framework/Core/test/test_AnalysisTask.cxx:175: �[1;31;49merror: in "AdaptorCompilation": check task3.inputs[0].binding == "Collisions_000" has failed [Collisions_001 != Collisions_000]�[0;39;49m

I guess the lines 171 and 175 of test_AnalysisTask.cxx should be also changed to 001 ?

@ddobrigk
Copy link
Contributor Author

Hi Ruben, yes, I am sorry, I missed that. I will take a look asap...

@jgrosseo jgrosseo enabled auto-merge (squash) December 15, 2022 15:13
@ddobrigk ddobrigk disabled auto-merge December 15, 2022 21:15
@ddobrigk
Copy link
Contributor Author

(fullCI is taking a bit, so for now, I disabled auto-merge so we can merge manually later when everyone is awake and then I immediately send a message informing of the need of the collision converter together with the merge...)

@jgrosseo
Copy link
Collaborator

It is green now? You want to do it on a Friday? The new tag will be Saturday morning...

@shahor02
Copy link
Collaborator

I would merge it, shall I?

@ddobrigk
Copy link
Contributor Author

@shahor02, Jan Fiete convinced me we better wait to merge on Sunday so that it goes on Monday's tag - I will ping again on Sunday ... Thanks!!

@chiarazampolli
Copy link
Collaborator

Hello,

Note we plan to use the tag from Monday for the apass2 of the 13.6 tev. If there is any chance that there are issues (otherwise it should have been merged already today, i guess you wanted to “save the weekend “), I would wait longer.

chiara

@jgrosseo jgrosseo merged commit bff504b into AliceO2Group:dev Dec 18, 2022
This was referenced Feb 16, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jgrosseo jgrosseo jgrosseo approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ddobrigk @shahor02 @jgrosseo @chiarazampolli