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

Make "piecewise" the default pdf interpolation #142

Merged
merged 3 commits into from
Feb 29, 2024
Merged

Conversation

hammannr
Copy link
Collaborator

This PR makes "piecewise" the default pdf interpolation method. Before, the default was defined in blueice to "linear", which is only in certain limits compatible with the BlueiceDataGenerator. The BlueiceDataGenerator is now only assigned if all sources have piecewise interpolation -- in the future we can add a suitable generator also for the linear interpolation case.

@hammannr hammannr added the bug Something isn't working label Feb 27, 2024
@hammannr hammannr requested review from kdund and dachengx February 27, 2024 17:31
@coveralls
Copy link

coveralls commented Feb 27, 2024

Pull Request Test Coverage Report for Build 8087518258

Details

  • 12 of 13 (92.31%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.007%) to 90.738%

Changes Missing Coverage Covered Lines Changed/Added Lines %
alea/models/blueice_extended_model.py 8 9 88.89%
Totals Coverage Status
Change from base Build 8082027034: 0.007%
Covered Lines: 1450
Relevant Lines: 1598

💛 - Coveralls

Copy link
Collaborator

@kdund kdund left a comment

Choose a reason for hiding this comment

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

Looks good for me-- any way to test?

@hammannr
Copy link
Collaborator Author

@kdund you could add pdf_interpolation_method="linear" in one of the example configs and try to load the model -- it should complain in the initialization step that it doesn't like it. If you add pdf_interpolation_method="piecewise" or nothing at all it should work fine.

Copy link
Collaborator

@kdund kdund left a comment

Choose a reason for hiding this comment

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

Checked, and works, thanks @hammannr

@hammannr hammannr merged commit 2de4201 into main Feb 29, 2024
7 checks passed
@hammannr hammannr deleted the interpolation_default branch February 29, 2024 06:26
@dachengx
Copy link
Collaborator

Hey @hammannr . Maybe you could try to squash and merge next time. Then it is easier to track the change from a PR in the GitHub history.

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.

4 participants