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

Incomplete signal patch treatment #12

Merged
merged 9 commits into from
Jul 12, 2024
Merged

Incomplete signal patch treatment #12

merged 9 commits into from
Jul 12, 2024

Conversation

jackaraz
Copy link
Member

@jackaraz jackaraz commented Jul 10, 2024

This PR improves patch set handling when patches do not include any information about certain channels.

Thanks to @Rav2 for pointing this out.

@jackaraz jackaraz linked an issue Jul 10, 2024 that may be closed by this pull request
@jackaraz jackaraz added the bug Something isn't working label Jul 10, 2024
* added separate remove list to remove channels if they are specifically set as remove, otherwise they will be included in the model
@jackaraz jackaraz marked this pull request as ready for review July 11, 2024 15:23
@jackaraz jackaraz added enhancement New feature or request and removed bug Something isn't working labels Jul 11, 2024
Copy link

@Rav2 Rav2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TEST 1

  1. Load data from ATLAS 1911.12606 analysis for EWKinos
  2. Create wrapper with the background and the first patch
  3. Get model and data for the 'apriori' ExpectationType
  4. The output:
Spey - WARNING: Some of the channels are not defined in the patch set, these channels will be kept in the statistical model. 
Spey - WARNING: If these channels are meant to be removed, please indicate them in the patch set.
Spey - WARNING: Please check the following channel(s): SR_eMLLa_Onelep1track_cuts, SR_eMLLb_Onelep1track_cuts, SR_eMLLc_Onelep1track_cuts, SR_eMLLd_Onelep1track_cuts, SR_eMLLe_Onelep1track_cuts, SR_eMLLf_Onelep1track_cuts, SRee_eMLLf_lowmet_deltaM_low_cuts, SRmm_eMLLh_lowmet_deltaM_high_cuts

TEST2

  1. Use the model from TEST1
  2. Calculate nLLs for ExpectationTypes: ['apriori', 'aposteriori', 'observed'] for mu=0 and mu=1
  3. Output:
spey (apriori): nLL_exp_mu0=258.82 nLL_exp_mu1=265.83
spey (aposteriori): nLL_exp_mu0=294.44 nLL_exp_mu1=296.38
spey: nLL_obs_mu0=294.44 nLL_obs_mu1=296.38

TEST3

  1. Repeat TEST1 for all files and patches from 1911.12606 analysis
  2. Observation: sometimes many SRs are missing in the patches
  3. Result: PASSED

COMMENTS AND SUGGESTIONS

  1. Overall, it works good.
  2. The warning messages are good, but wouldn't it be better to merge them into one, or two (first merged with second) rather than print 3?
  3. When user loads multiple patches, like I do in TEST3, or when we want to calculate limit, it would be helpful to print the name of the patch in the warning (a check would be needed if there is a name in the metadata), because different patches (mass points) might be missing information for various channels.
  4. An optional flag would be nice to silence these warnings if we know what we are doing.

@jackaraz
Copy link
Member Author

TEST 1

  1. Load data from ATLAS 1911.12606 analysis for EWKinos
  2. Create wrapper with the background and the first patch
  3. Get model and data for the 'apriori' ExpectationType
  4. The output:
Spey - WARNING: Some of the channels are not defined in the patch set, these channels will be kept in the statistical model. 
Spey - WARNING: If these channels are meant to be removed, please indicate them in the patch set.
Spey - WARNING: Please check the following channel(s): SR_eMLLa_Onelep1track_cuts, SR_eMLLb_Onelep1track_cuts, SR_eMLLc_Onelep1track_cuts, SR_eMLLd_Onelep1track_cuts, SR_eMLLe_Onelep1track_cuts, SR_eMLLf_Onelep1track_cuts, SRee_eMLLf_lowmet_deltaM_low_cuts, SRmm_eMLLh_lowmet_deltaM_high_cuts

TEST2

  1. Use the model from TEST1
  2. Calculate nLLs for ExpectationTypes: ['apriori', 'aposteriori', 'observed'] for mu=0 and mu=1
  3. Output:
spey (apriori): nLL_exp_mu0=258.82 nLL_exp_mu1=265.83
spey (aposteriori): nLL_exp_mu0=294.44 nLL_exp_mu1=296.38
spey: nLL_obs_mu0=294.44 nLL_obs_mu1=296.38

TEST3

  1. Repeat TEST1 for all files and patches from 1911.12606 analysis
  2. Observation: sometimes many SRs are missing in the patches
  3. Result: PASSED

COMMENTS AND SUGGESTIONS

The warning messages are good, but wouldn't it be better to merge them into one or two (first merged with second) rather than print 3?

That's intentional, actually; otherwise, everything gets into each other and makes things avoidable. I want it to scream to the user.

An optional flag would be nice to silence these warnings if we know what we are doing.

That is already there see the documentation.

When user loads multiple patches, like I do in TEST3, or when we want to calculate limit, it would be helpful to print the name of the patch in the warning (a check would be needed if there is a name in the metadata), because different patches (mass points) might be missing information for various channels.

Patch sets does not necessarily have a unique name, it will work even if you give the same name to all so that wont change anything.

Copy link

@Rav2 Rav2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seal of approval.

@jackaraz jackaraz merged commit 33dae30 into main Jul 12, 2024
1 check passed
@jackaraz jackaraz deleted the incomplete_patch branch July 12, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expected background yields when signal patch is not complete
2 participants