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

Bug fix/time window exclusions #8

Merged
merged 26 commits into from
May 31, 2024

Conversation

mhuen
Copy link
Collaborator

@mhuen mhuen commented Nov 4, 2022

Previously, when excluding time window exclusions, the individual PDFs of each mixture model component were re-normalized individually. This, however, is incorrect as this will modify the shape of the combined PDF. Instead, the entire mixture PDF has to be re-normalized by the total dom_cdf_exclusions_sum.

Note: this bugfix is incompatible with models trained in previous versions. To indicate this, the version number is increased.

…y for each of the components, which is incorrect and leads to bias
…y for each of the components, which is incorrect and leads to bias
…y for each of the components, which is incorrect and leads to bias
@mhuen mhuen changed the base branch from master to CollectBreakingChanges May 29, 2024 13:02
@codecov-commenter
Copy link

codecov-commenter commented May 29, 2024

Codecov Report

Attention: Patch coverage is 0% with 57 lines in your changes are missing coverage. Please review.

Please upload report for BASE (CollectBreakingChanges@769de28). Learn more about missing BASE report.

Files Patch % Lines
...manager/reconstruction/modules/visualize_pulses.py 0.00% 15 Missing ⚠️
egenerator/ic3/visualization.py 0.00% 11 Missing ⚠️
egenerator/model/source/noise/default.py 0.00% 9 Missing ⚠️
egenerator/model/source/base.py 0.00% 8 Missing ⚠️
egenerator/loss/default.py 0.00% 6 Missing ⚠️
egenerator/model/multi_source/base.py 0.00% 4 Missing ⚠️
egenerator/model/source/cascade/default.py 0.00% 4 Missing ⚠️
Additional details and impacted files
@@                    Coverage Diff                    @@
##             CollectBreakingChanges       #8   +/-   ##
=========================================================
  Coverage                          ?   20.58%           
=========================================================
  Files                             ?       87           
  Lines                             ?     9482           
  Branches                          ?     1727           
=========================================================
  Hits                              ?     1952           
  Misses                            ?     7413           
  Partials                          ?      117           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mhuen mhuen marked this pull request as ready for review May 29, 2024 14:10
@mhuen
Copy link
Collaborator Author

mhuen commented May 31, 2024

Note on the failed model prediction test: this test is expected to fail at the moment since an incompatibility check was introduced that triggers when trying to run with models of older software versions. Once all breaking changes are included in the branch CollectBreakingChanges, I will train a new model and update the test data to fix these unit tests.

@mhuen mhuen merged commit dedb31f into CollectBreakingChanges May 31, 2024
0 of 4 checks passed
@mhuen mhuen deleted the BugFix/TimeWindowExclusions branch May 31, 2024 07:20
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.

2 participants