-
Notifications
You must be signed in to change notification settings - Fork 57
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
Removed independent volume check in vpf.py #238
Conversation
Following the discussion in issue manodeep#237 and after conversations with LGarrison, I've completely removed the check that confirmed that the volume inhabited by the dropped spheres was less than the volume of the sample. This check was unnecessary for the VPF to run, and actively discouraged the oversampling of the sample volume with test spheres that one needs to precisely measure the VPF at a given radius.
Thanks! The CI failures are not your fault, they're on the backend. I've updated master with the fix, can you re-merge master into your branch? You may have to add this repo as an upstream remote, if it's not there already:
or something close to that! Also, the volume calculation above your edits can probably be removed too. |
Without the check of independent volumes, there's no need to overtly calculate the volume the samples inhabit.
@laperezNYC Thanks for the PR! Will you please add an entry to the @lgarrison Did the astropy bot run the usual checks on this PR? Not seeing it... |
Yeah, I was wondering about the bot too. Don't see it... |
Saw that the bot was requesting some additional permissions (read + write to checks, read + write to members) - granted those permissions. Hopefully, that causes the bot to run at the next commit + update |
Updated 2.4.0 with this new enhancement to theory.vpf
I promise I can type!
@laperezNYC Tweaked the wording a bit. If this looks acceptable to you, then I will merge in the PR
@laperezNYC Had not realised that this PR was still outstanding! Thanks for the contribution |
Following the discussion in issue #237 and after conversations with LGarrison, I've completely removed the check that confirmed that the volume inhabited by the dropped spheres was less than the volume of the sample. This check was unnecessary for the VPF to run, and actively discouraged the oversampling of the sample volume with test spheres that one needs to precisely measure the VPF at a given radius.