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

reset histograms to min of zero on the fly #1164

Merged
merged 6 commits into from
Nov 15, 2024
Merged

Conversation

rnugent3
Copy link
Contributor

@rnugent3 rnugent3 commented Nov 5, 2024

I created a fix that shifted a histogram if we obtained a zero-valued result after construction of a histogram to allow the Histogram Min to be 0, not just the sample min. The fix requires us to shift the histogram up to add the required number of bins to reach zero, and put the leftward most edge of the histogram right at 0. This means that pre-shift bins and post-shift bins will partially overlap, and those partials need to be allocated out correctly based upon the fraction of the observations in a pre-shift bin that overlaps with the post-shift bin, where the fraction is determined with the assumption of a uniformly distributed bin. I think the majority of the histogram math works that way.

This fix is only reached if the min is 0. If the min goes below 0, so will the histogram.

This fix produces a really nice result. Prior to the fix, the histograms gave us really wonky left tails. See for example:
image
After the fix, we have a logical left tail:
image

A unit test is included to demonstrate that the added logic works as expected.

@rnugent3 rnugent3 linked an issue Nov 5, 2024 that may be closed by this pull request
@rnugent3
Copy link
Contributor Author

rnugent3 commented Nov 7, 2024

Security systems are preventing me from running a unit test locally so I pushed it here

@Brennan1994 Brennan1994 merged commit c4c757b into main Nov 15, 2024
1 check passed
@Brennan1994 Brennan1994 deleted the zero-based-histograms branch November 15, 2024 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide zero bound on damage hists
2 participants