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

Forbid prexing every key when adapt_likelihood_config_for_blueice #170

Merged
merged 8 commits into from
Jun 27, 2024

Conversation

FaroutYLq
Copy link
Contributor

This is likely a bug introduced in #169. The original implementation was trying to prefix every field, as long as it happens to be an existing file after prefixing. You can see one example that this will lead to a problem.

Let's say you have defined the following sources in your statistical model yaml file:

      - name: b8
        histname: template
        parameters:
          - b8_rate_multiplier
          - t_ly
          - t_qy
        named_parameters:
          - t_ly
          - t_qy
        template_filename: cevns/template_XENONnT_sr0_cevns_cevns_tly_{t_ly:.1f}_tqy_{t_qy:.1f}.h5

      - name: ac
        histname: template
        parameters:
          - ac_rate_multiplier
        template_filename: ac/template_XENONnT_sr0_ac_cevns.h5

in this example, with current alea it will complain a bug. In general, as long as your source name coincidence with a path name after prefixing, it will give an issue.

  File "/home/yuanlq/software/blueice/blueice/likelihood.py", line 365, in _kwargs_to_settings
    raise InvalidParameter("%s is not a known shape or rate parameter!" % k)
blueice.exceptions.InvalidParameter: ac_rate_multiplier is not a known shape or rate parameter!

The issue took place inside adapt_likelihood_config_for_blueice. Before prefixing here, source looks like

{'name': 'ac', 'histname': 'template', 'parameters': ['ac_rate_multiplier'], 'template_filename': 'ac/template_XENONnT_sr0_ac_cevns.h5', 'templatename': '/project2/lgrandi/light_wimp/templates/ambe_ybe_combined/ac/template_XENONnT_sr0_ac_cevns.h5'}

After, it becomes:

{'name': '/project2/lgrandi/light_wimp/templates/ambe_ybe_combined/ac', 'histname': 'template', 'parameters': ['ac_rate_multiplier'], 'template_filename': '/project2/lgrandi/light_wimp/templates/ambe_ybe_combined/ac/template_XENONnT_sr0_ac_cevns.h5', 'templatename': '/project2/lgrandi/light_wimp/templates/ambe_ybe_combined/ac/template_XENONnT_sr0_ac_cevns.h5'}

You can see that now the source name makes no sense.

In this patch solution, we try to avoid config keys that will in no way be interpreted as a path or filename, by making them into ignore_keys.

@FaroutYLq FaroutYLq added the bug Something isn't working label Jun 26, 2024
@FaroutYLq FaroutYLq requested review from dachengx and hammannr June 26, 2024 23:55
@FaroutYLq FaroutYLq self-assigned this Jun 26, 2024
@coveralls
Copy link

coveralls commented Jun 26, 2024

Pull Request Test Coverage Report for Build 9688342366

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 11 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 92.149%

Files with Coverage Reduction New Missed Lines %
alea/utils.py 11 89.24%
Totals Coverage Status
Change from base Build 9688264437: 0.0%
Covered Lines: 1561
Relevant Lines: 1694

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 27, 2024

Pull Request Test Coverage Report for Build 9688343473

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 11 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 92.149%

Files with Coverage Reduction New Missed Lines %
alea/utils.py 11 89.24%
Totals Coverage Status
Change from base Build 9688264437: 0.0%
Covered Lines: 1561
Relevant Lines: 1694

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 27, 2024

Pull Request Test Coverage Report for Build 9688343473

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
  • 11 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 92.149%

Files with Coverage Reduction New Missed Lines %
alea/utils.py 11 89.24%
Totals Coverage Status
Change from base Build 9688264437: 0.0%
Covered Lines: 1561
Relevant Lines: 1694

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 27, 2024

Pull Request Test Coverage Report for Build 9688353716

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 14 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.06%) to 92.208%

Files with Coverage Reduction New Missed Lines %
alea/utils.py 14 89.24%
Totals Coverage Status
Change from base Build 9688264437: 0.06%
Covered Lines: 1562
Relevant Lines: 1694

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 27, 2024

Pull Request Test Coverage Report for Build 9688354594

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 14 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 92.149%

Files with Coverage Reduction New Missed Lines %
alea/utils.py 14 89.24%
Totals Coverage Status
Change from base Build 9688264437: 0.0%
Covered Lines: 1561
Relevant Lines: 1694

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 27, 2024

Pull Request Test Coverage Report for Build 9688366339

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.06%) to 92.208%

Totals Coverage Status
Change from base Build 9688264437: 0.06%
Covered Lines: 1562
Relevant Lines: 1694

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 27, 2024

Pull Request Test Coverage Report for Build 9688365224

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 14 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 92.149%

Files with Coverage Reduction New Missed Lines %
alea/utils.py 14 89.24%
Totals Coverage Status
Change from base Build 9688264437: 0.0%
Covered Lines: 1561
Relevant Lines: 1694

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 27, 2024

Pull Request Test Coverage Report for Build 9688404272

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 92.149%

Totals Coverage Status
Change from base Build 9688264437: 0.0%
Covered Lines: 1561
Relevant Lines: 1694

💛 - Coveralls

Copy link
Collaborator

@dachengx dachengx left a comment

Choose a reason for hiding this comment

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

Thanks @FaroutYLq

@dachengx dachengx merged commit 19cf822 into main Jun 27, 2024
7 checks passed
@dachengx dachengx deleted the fix_prefix_file_path branch June 27, 2024 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants