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

Get expectation values per likelihood term #106

Merged
merged 9 commits into from
Nov 8, 2023
Merged

Conversation

hammannr
Copy link
Collaborator

@hammannr hammannr commented Nov 8, 2023

This PR adds additional functionality to the get_expectation_values method in the BlueiceExtendedModel. By default, the expectation values for the same source are summed over all likelihood terms. This remains to be the default, however now you can also get a dict of a dict, which contains the expectation values for each lieklihood term individually.

Example:

from alea import BlueiceExtendedModel
model = BlueiceExtendedModel.from_config("unbinned_wimp_statistical_model.yaml")

print(model.get_expectation_values())
# out: {'wimp': 12.0, 'er': 440.0}

print(model.get_expectation_values(per_likelihood_term=True))
# {'sr0': {'er': 40.0, 'wimp': 2.0}, 'sr1': {'er': 400.0, 'wimp': 10.0}}

@hammannr hammannr added the enhancement New feature or request label Nov 8, 2023
Copy link

github-actions bot commented Nov 8, 2023

Pull Request Test Coverage Report for Build 6801701346

  • 8 of 8 (100.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.05%) to 91.655%

Files with Coverage Reduction New Missed Lines %
alea/model.py 1 85.64%
Totals Coverage Status
Change from base Build 6772566926: -0.05%
Covered Lines: 1351
Relevant Lines: 1474

💛 - Coveralls

@hammannr hammannr requested review from dachengx and kdund November 8, 2023 14:30
@hammannr hammannr marked this pull request as ready for review November 8, 2023 14:30
@hammannr
Copy link
Collaborator Author

hammannr commented Nov 8, 2023

The pre-commit doesn't like the way I change names I think. But I kind of prefer changing the type of ret instead of having to returns in this function.. What do you think?

@kdund
Copy link
Collaborator

kdund commented Nov 8, 2023

To me it looks fine-- mypy is not happy though (I think the overloaded type of ret?

@hammannr
Copy link
Collaborator Author

hammannr commented Nov 8, 2023

Yes exactly @kdund but I think it's still nicer to overload this than to have to returns of the function or not?

@kdund
Copy link
Collaborator

kdund commented Nov 8, 2023

Any way you can ask mypy to skip those lines?

@kdund
Copy link
Collaborator

kdund commented Nov 8, 2023

Alternately, you could have type hints on return_per_ll and return_sum separately and return either depending?

@hammannr
Copy link
Collaborator Author

hammannr commented Nov 8, 2023

Actually, I think the cleanest way would be to always return a dict of dicts and in the default case it only has one key "total". But I kind of hate to make the default more cumbersome to use... So I think I'll just overload this and make mypy ignore the line.

@hammannr hammannr merged commit d14e8ef into main Nov 8, 2023
@hammannr hammannr deleted the expectation_per_ll branch November 8, 2023 17:21
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.

3 participants