XENONnT / straxen

Streaming analysis for XENON
BSD 3-Clause "New" or "Revised" License
20 stars 32 forks source link

Let xedocs to handle avg seg and seg partitioning #1371

Closed GiovanniVolta closed 5 months ago

GiovanniVolta commented 5 months ago

What does the code in this PR do / what does it improve?

Changed how to handle the TPC partition for AB and CD SEG and avg SEG.

Relevant for xedocs PR#120 and correction PR#273

Can you briefly describe how it works?

The partition AB is distinguished from CD based on a linear and a radial cut on the (xy) plane. For the two partitions, we have different average SE gains.

We have add two schemas, one for the definition of AB/CD and one for the avg_se_gain. Here they are implemeneted.

Can you give a minimal working example (or illustrate with a figure)?

Please include the following if applicable:

GiovanniVolta commented 5 months ago

Hey @yuema137 and @dachengx, do you have a suggestion on verifying that nothing changed for SR0? From the test performed in correction PR#274 the SR0 values of avg SEG and SEg partitioning are good!

yuema137 commented 5 months ago

@GiovanniVolta Thanks for the PR! It looks fine for me as the dates and values are correct. But I'm wondering should we also replace the avg SEG? For now only the partitions are replaced.

GiovanniVolta commented 5 months ago

For the following test you have to be on correction seg_partition branch and on xedocs se_gain_partition_schema branch

immagine immagine immagine immagine

Note that for these test I have specified where to fetch the data. So basically, this is what the new global version will look like.

I have also tested the online context, and it works well for the partitioning definition. For AB and CD values, we don't have to worry since it is handled by the plugin itself here (this should reply to your first question @yuema137)

immagine

GiovanniVolta commented 5 months ago

Another comment: I understand that the tests may fail because of not updated xedocs and other relevant repos. Could you provide the dependencies between them and point out which PRs we should use to test together? Thanks a lot

Thank you, @yuema137, for the comment. So these PR should be tested with correction seg_partition branch and on xedocs se_gain_partition_schema Actually, I think now it does not matter anymore since I have changed the default version of single_electron_gain_partition Mmmm not sure what it is going on Sorry I forgot to push a commit. My bad

LuisSanchez25 commented 5 months ago

I thought we discussed yesterday that github does not have access to our mongo database so the URLs could not be set to xedocs?

coveralls commented 5 months ago

Coverage Status

coverage: 91.537% (+0.02%) from 91.517% when pulling 8b9991c0f2fcdc22f3e5f946ac7d6652b48ca317 on avg_seg_and_seg_partitioning into e555c7dcada2743d2ea627ea49df783e9dba40e3 on master.

yuema137 commented 5 months ago

@GiovanniVolta Thanks for the explanation. So, in my understanding, the `single_electron_gain_partition' config already contains the SEG values. https://github.com/XENONnT/straxen/blob/194a44e32aa2e6a0638ef6463e6f6c48afb66201/straxen/plugins/events/corrected_areas.py#L90 In that case, to avoid confusion, should we remove those two configs? It seems that they are set by CMT

https://github.com/XENONnT/straxen/blob/194a44e32aa2e6a0638ef6463e6f6c48afb66201/straxen/plugins/events/corrected_areas.py#L58

https://github.com/XENONnT/straxen/blob/194a44e32aa2e6a0638ef6463e6f6c48afb66201/straxen/plugins/events/corrected_areas.py#L67

The current practice looks like a mixture of CMT and xedocs which is not ideal. So could you look into this? Thanks a lot

GiovanniVolta commented 5 months ago

Hey @yuema137, No, no, the single_electron_gain_partition now only takes care of linear and circular selection for defining the AB and CD partition. Similarly, the avg_se_gain and se_gain take care only of their respective variables. There is no nested indexing anymore. Sorry for the confusion. Please have a look at the correction PR#273 to further understand the two new schemas. I have also updated the PR description. My bad; I should have done this before.

GiovanniVolta commented 5 months ago

@flammhead asked me if these changes and the release of a new version of xedocs will affect the data lineage. My naive answer is only above the event level. The Strax/staxen version has not been changed, and the lineage does not care about the xedocs version. @yuema137 , @dachengx is this correct?

GiovanniVolta commented 5 months ago

The relevant PRs in xedocs and correction have been merged! We are good to go imo