[FIT] Follow data format changes in O2 #8344#1092
Merged
knopers8 merged 2 commits intoAliceO2Group:masterfrom Mar 17, 2022
Merged
[FIT] Follow data format changes in O2 #8344#1092knopers8 merged 2 commits intoAliceO2Group:masterfrom
knopers8 merged 2 commits intoAliceO2Group:masterfrom
Conversation
knopers8
approved these changes
Mar 15, 2022
Contributor
Author
|
Hi @knopers8 thanks for approving. But I'm not sure how to deal with the CI tests in this case. This is a chicken-and-egg problem. I mean the CI tests of this PR in QC require that the O2 PR #8344 is merged. On the other hand, the CI tests in O2 need this PR to be merged first. I'll ping also @shahor02 for a second opinion, but I think it would be best to merge the QC first, even with building errors, simply because it is a much smaller PR, involving only 8 files, as opposed to the PR in O2, which is a major one involving over 100 files. Another approach could be to split this QC PR into two parts
What do you think? |
Contributor
|
Hi @mslupeck |
Contributor
Author
|
Hi @shahor02, I think there is a misunderstanding. I've created this QC PR only because the O2 CI tests fail (the O2 tests apparently also require a successful QC build). So if I restore the O2 files that are needed for this QC PR to pass, there is no need for this QC PR. |
Contributor
|
@mslupeck The AliceO2Group/AliceO2#8344 fails in QC compilation at If these old headers would be available, why the QC should fail? |
Contributor
Author
|
Ah... so you mean that I should just restore the old headers without removing the new ones and without restoring the functionality of the old ones? Yes... that could work. I'll try it out. Thanks for the suggestion! |
Contributor
|
yes |
auto-merge was automatically disabled
March 15, 2022 20:55
Head branch was pushed to by a user without write access
Contributor
Author
|
So I don't see how to do it on a reasonable time/effort scale following @shahor02 suggestion. The problem is that missing BCData.h header is just the tip of an iceberg. The next error comes from the ChannelData, in which all the data members have changed names (to follow FT0's convention). So for now, I propose to disable FV0 QC entirely. It should get the O2 PR through and then the FV0 QC could be reenabled (we are not planning to use FV0 QC for another 2 weeks or so). |
Collaborator
|
FV0 seems to be enabled also in asynchronous reconstructions, are you sure you want to disable it for two weeks? |
Contributor
Author
|
@knopers8: I've just spoken with @chiarazampolli and it seems we still have a few weeks before everyone will be ready for apass4. And in case I'm late with reenabling the FV0 QC, she agreed to exclude the fv0 qc json manually. So yes, I'm sure about disabling the FV0 QC. |
Contributor
|
Hi @mslupeck , I did not really say "few weeks", what I meant is that we are not ready to start right away, I have no clear ETA, but it won't be an issue to disable FV0 QC in the apass4, as you wrote. Chiara |
Contributor
Author
|
OK - sorry for the misunderstanding about the schedule of apass4. It's clear it is not settled yet. In any case, the exact timing of apass4 is not blocking this PR from merging. |
knopers8
approved these changes
Mar 17, 2022
Barthelemy
pushed a commit
that referenced
this pull request
Mar 21, 2022
* [FIT] Follow data format changes in O2 #8344 * Temporarily disable FV0 QC Co-authored-by: Maciej Slupecki <maciej.slupecki@cern.ch>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is necessary for the CI tests in O2 PR #8344 to pass