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

Proper combining of nsamples for psuedo-Stokes to preserve proper noise variance #392

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

jsdillon
Copy link
Member

@jsdillon jsdillon commented Feb 5, 2024

This PR resolved #391

…serve proper noise variance""

This reverts commit 36b1e7d.
Copy link

codecov bot commented Feb 5, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (36b1e7d) 0.00% compared to head (dfb0969) 95.99%.

Additional details and impacted files
@@            Coverage Diff            @@
##           main     #392       +/-   ##
=========================================
+ Coverage      0   95.99%   +95.99%     
=========================================
  Files         0       17       +17     
  Lines         0     6118     +6118     
=========================================
+ Hits          0     5873     +5873     
- Misses        0      245      +245     
Flag Coverage Δ
unittests 95.99% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Copy link

@mwilensky768 mwilensky768 left a comment

Choose a reason for hiding this comment

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

The simulations show that it's clearly predicting the right variance, and moreover it's all mathematically sound. While I agree that it doesn't quite agree with the definition of nsamples, it's the right thing to do for variance prediction and that is all we use nsamples for in pspec as far as I know.

I've done a quick check to see that if one or both of these entries are 0.0 (that is, a numpy array with 0.0 as entries, not the builtin float 0.0), this syntax returns 0.0, which is as desired. Additionally nan's beget nans.

I approve.

@jsdillon jsdillon merged commit 7f101f1 into main Feb 7, 2024
8 of 9 checks passed
@jsdillon jsdillon deleted the pstokes-nsamples branch February 7, 2024 19:05
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.

Pseudo-Stokes Nsamples Accounting
2 participants