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

Remove duplication of calls to resampling functions #72

Closed
wleoncio opened this issue Dec 13, 2021 · 5 comments
Closed

Remove duplication of calls to resampling functions #72

wleoncio opened this issue Dec 13, 2021 · 5 comments
Labels
enhancement New feature or request

Comments

@wleoncio
Copy link
Member

The resampling procedure should be its only funciton. Instead, it is performed on rtrunc() and then again on sampleFromUntruncated().

@wleoncio wleoncio added the enhancement New feature or request label Dec 13, 2021
@wleoncio
Copy link
Member Author

One promising solution involves determining a larger sample size than the user requested (if a and b are not the defaults) and dropping the truncated values from that. That should generate very little extra code footprint, basically just a formula for the new n, ideally something directly proportional to how extreme the requested configuration is. For example:

  1. buffer_n = n * 100 / (b - a)
  2. buffer_n = n * 100 / P(a < X < b)

@rho62
Copy link
Contributor

rho62 commented Feb 20, 2022 via email

@wleoncio
Copy link
Member Author

Hi Rene, thanks for chiming in! I couldn't find your code, I suppose it was attached to your reply (which GitHub discards). Could you post your solution on https://gist.github.com/ and add the link to it here? You can also send it to me by e-mail and I'll do that, noproblem. :)

@wleoncio wleoncio changed the title DRY resampling Remove duplication of calls to resampling functions Feb 21, 2022
@wleoncio
Copy link
Member Author

Renamed this issue to clarify the difference between what is handled here (i.e., the WET coding that calls sampleFromUntruncated() more often than it should) and what is handled on #77 and #78, which is the analytical solution to the sampling itself. Solving the latter two will probably also end up solving this issue, however.

@wleoncio wleoncio added this to the MVP 1.2.0 milestone Apr 13, 2023
@wleoncio
Copy link
Member Author

Since #77 and #78 have been implemented, this older procedure might be superseded. It currently is still the default, but that can change at any time. The only advantage of the older procedure is sample-equivalence to their untruncated counterparts (given the same seed). Unless this equivalence is essential, working on this issue might not be worth it.

Closing for now, revisiting in the future if necessary.

@wleoncio wleoncio closed this as not planned Won't fix, can't repro, duplicate, stale Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants