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

fix: make non-uniform rebinning work for Weight() and friends #972

Merged
merged 4 commits into from
Jan 29, 2025

Conversation

Saransh-cpp
Copy link
Member

@Saransh-cpp Saransh-cpp commented Jan 23, 2025

Fixes #971

Uses View's addition when adding the views during non-uniform rebinning.

@github-actions github-actions bot added the needs changelog Might need a changelog entry label Jan 23, 2025
@matthewfeickert
Copy link
Member

cc @andrzejnovak

@andrzejnovak
Copy link
Member

image

Works (and propagates to hist!). Thanks a lot!

@matthewfeickert
Copy link
Member

@Saransh-cpp I'll let you merge in the event there's any final small things you wanted to get in here.

@Saransh-cpp
Copy link
Member Author

Thanks! I don't have merge/write access to boost-histogram. Please feel free to merge :)

@matthewfeickert
Copy link
Member

Ah okay I'll also ask @henryiii for review and merge then.

@henryiii
Copy link
Member

Are all the fields addable? That is, for every type, can you just directly add the fields? For example, for the Mean storage, this is the definition of __iadd__:

mean& operator+=(const mean& rhs) noexcept {
if(rhs.count == 0)
return *this;
const auto mu1 = value;
const auto mu2 = rhs.value;
const auto n1 = count;
const auto n2 = rhs.count;
count += rhs.count;
value = (n1 * mu1 + n2 * mu2) / count;
_sum_of_deltas_squared += rhs._sum_of_deltas_squared;
_sum_of_deltas_squared
+= n1 * (value - mu1) * (value - mu1) + n2 * (value - mu2) * (value - mu2);
return *this;
}

I think we can just use this, though, by exposing it via a ufunc. I'm looking at this soon, but got distracted updating and fixing things in #973.

@henryiii henryiii merged commit 3919b73 into scikit-hep:develop Jan 29, 2025
17 checks passed
@henryiii
Copy link
Member

Fixed. Thanks!

@matthewfeickert
Copy link
Member

Thanks @henryiii and @Saransh-cpp!

Would it be possible to get a patch release with this out?

@Saransh-cpp Saransh-cpp deleted the saransh/issue-971 branch January 29, 2025 20:10
@henryiii
Copy link
Member

1.5.1 is out. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs changelog Might need a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] non-uniform rebinning does not work with Weight() storage
4 participants