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 constraints for SubSetSimulation's proposal pdf #132

Merged
merged 2 commits into from
Sep 13, 2023
Merged

Fix constraints for SubSetSimulation's proposal pdf #132

merged 2 commits into from
Sep 13, 2023

Conversation

andreaperin
Copy link
Collaborator

@andreaperin andreaperin commented Sep 13, 2023

SubSet simulation's proposal pdf has to be symmetric, centered in 0 and its variance should lower then 2 for avoiding too large chain's steps.

  • A check for 0 mean pdf is added
  • A warning will be thrown when the variance is greater than 2

Closes #130.

        - centered in 0
        - variance lower than 2
@andreaperin andreaperin changed the title Fix constrains for SubSetSimulation's proposal pdf Fix constraints for SubSetSimulation's proposal pdf Sep 13, 2023
@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Patch coverage: 85.71% and project coverage change: -0.11% ⚠️

Comparison is base (8429fcb) 99.06% compared to head (49f6661) 98.95%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #132      +/-   ##
==========================================
- Coverage   99.06%   98.95%   -0.11%     
==========================================
  Files          23       23              
  Lines         852      858       +6     
==========================================
+ Hits          844      849       +5     
- Misses          8        9       +1     
Files Changed Coverage Δ
src/simulations/subset.jl 99.17% <85.71%> (-0.83%) ⬇️

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

@@ -30,6 +30,10 @@ struct SubSetSimulation <: AbstractSubSetSimulation
skewness(proposal) != 0.0 && error("proposal must be a symmetric distribution")
mean(proposal) != median(proposal) &&
error("proposal must be a symmetric distribution")
mean(proposal) != 0 && error("proposal must be centered in 0")
if var(proposal) ≥ 2
@warn "proposal pdf with large variance can be unefficient. Use a variance lower than 2."
Copy link
Owner

Choose a reason for hiding this comment

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

"A proposal pdf with large variance (≥ 2) can be inefficient."

Copy link
Owner

Choose a reason for hiding this comment

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

If you want you can make the other errors full sentences while your at it.

@@ -30,6 +30,10 @@ struct SubSetSimulation <: AbstractSubSetSimulation
skewness(proposal) != 0.0 && error("proposal must be a symmetric distribution")
mean(proposal) != median(proposal) &&
error("proposal must be a symmetric distribution")
mean(proposal) != 0 && error("proposal must be centered in 0")
if var(proposal) ≥ 2
@warn "proposal pdf with large variance can be unefficient. Use a variance lower than 2."
Copy link
Owner

Choose a reason for hiding this comment

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

If you want you can make the other errors full sentences while your at it.

@FriesischScott
Copy link
Owner

Also add a source for the variance < 2 statement please.

variance. Change error statement in "full sentence".
@FriesischScott FriesischScott merged commit 8a95dbf into FriesischScott:master Sep 13, 2023
7 checks passed
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.

SubSet returns NaN cov for 1 rv
2 participants