Skip to content

Use Lifetime::QA for sampled quantities#7383

Merged
ktf merged 2 commits intoAliceO2Group:devfrom
ktf:datasampling-qa
Oct 22, 2021
Merged

Use Lifetime::QA for sampled quantities#7383
ktf merged 2 commits intoAliceO2Group:devfrom
ktf:datasampling-qa

Conversation

@ktf
Copy link
Copy Markdown
Member

@ktf ktf commented Oct 20, 2021

This will make sure that sampled data will not be used to do counting
of in-fly timeframes.

@davidrohr
Copy link
Copy Markdown
Collaborator

Is it safe to just overwrite it? Shouldn't we request the developer to set it, and give a failure if not? Otherwise I think this can be confusing?

@ktf
Copy link
Copy Markdown
Member Author

ktf commented Oct 20, 2021

@knopers8 @Barthelemy in order to correctly account for how many timeframes have been processed, we need to have all OutputSpec which are associated to sampled data marked as Lifetime::QA. While this will not change how the data is processed, it will make sure it will not be accounted to consider a timeframe "complete" (because only a fraction of the timeframes have it, by definition).

Do you see any problems with doing it?

@ktf
Copy link
Copy Markdown
Member Author

ktf commented Oct 20, 2021

@davidrohr see the other message. It Lifetime::Timeframe and Lifetime::QA should be identical in terms of processing, QA simply means that the data is sampled or created once in a while (like in the case of Analysis histograms).

@davidrohr
Copy link
Copy Markdown
Collaborator

Then why do we call it Lifetime::QA? Can we rename it to Lifetime::Sampled or something like that?

@ktf
Copy link
Copy Markdown
Member Author

ktf commented Oct 20, 2021

That's what I called it historically at the dawn of DPL. I am happy to rename it, however I do not like "Sampled" much, since it needs to apply also for histograms, which fit "Accumulated" more than "Sampled". Maybe "Occasional"?

@davidrohr
Copy link
Copy Markdown
Collaborator

In some sense I would call it Optional, but we have optional already for another purpose. What is actually exactly the difference?
How about sporadic / irregular / arbitrary / selective ?

@Barthelemy
Copy link
Copy Markdown
Collaborator

Hi,
@knopers8 is absent till next Monday and I think that he is the one who should give his green light. I propose to wait for him.

@davidrohr
Copy link
Copy Markdown
Collaborator

Hi @Barthelemy : this is actually on the critical path for commissioning, so I would not wait till Monday. Worst case we can revert. But it seems this breaks the DataSampling ctest. That must obviously be fixed.

@ktf
Copy link
Copy Markdown
Member Author

ktf commented Oct 21, 2021

Are you sure you are on the right branch?

@Barthelemy
Copy link
Copy Markdown
Collaborator

Hi @Barthelemy : this is actually on the critical path for commissioning, so I would not wait till Monday. Worst case we can revert. But it seems this breaks the DataSampling ctest. That must obviously be fixed.

Ok, I think that I fixed some of the failing tests. Although, @knopers8 will have to review it when he is back.

However for lines 112 and 113 I don't know how to fix it. If I put a lifetime, then the subspec is wrong, if I don't put it, it fails.

This will make sure that sampled data will not be used to do counting
of in-fly timeframes.
@ktf ktf force-pushed the datasampling-qa branch from d6ddd62 to 871bd81 Compare October 21, 2021 11:17
@ktf
Copy link
Copy Markdown
Member Author

ktf commented Oct 21, 2021

I fixed the remaining tests.

@davidrohr
Copy link
Copy Markdown
Collaborator

The test is still failing:

 58/401 Test  #67: test_DataSampling_test_DataSampling ..................................................***Failed    0.14 sec

@Barthelemy
Copy link
Copy Markdown
Collaborator

ok, forgot this one.

@davidrohr
Copy link
Copy Markdown
Collaborator

ok, forgot this one.

and it is not the DDS test :)

@Barthelemy
Copy link
Copy Markdown
Collaborator

hopefully it is ok now

@ktf
Copy link
Copy Markdown
Member Author

ktf commented Oct 22, 2021

So it seems ok now. @Barthelemy Can we merge?

@ktf ktf merged commit 12f7dfa into AliceO2Group:dev Oct 22, 2021
@ktf ktf deleted the datasampling-qa branch October 22, 2021 07:04
@knopers8
Copy link
Copy Markdown
Collaborator

I think this should be fine, but I will immediately contact you if I see something not working anymore.

ezradlesser pushed a commit to ezradlesser/AliceO2 that referenced this pull request Dec 1, 2021
This will make sure that sampled data will not be used to do counting
of in-fly timeframes.

Co-authored-by: Barthelemy <barthelemy.von.haller@cern.ch>
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.

4 participants