Skip to content

Commit

Permalink
Removed independent volume check in vpf.py (#238)
Browse files Browse the repository at this point in the history
* Removed independent volume check in vpf.py

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.

* Update vpf.py

Without the check of independent volumes, there's no need to overtly calculate the volume the samples inhabit.

* Update CHANGES.rst

Updated 2.4.0 with this new enhancement to theory.vpf

* Update CHANGES.rst

I promise I can type!

* Tweaked wording

@laperezNYC Tweaked the wording a bit. If this looks acceptable to you, then I will merge in the PR

Co-authored-by: Manodeep Sinha <[email protected]>
  • Loading branch information
laperezNYC and manodeep authored Jan 5, 2021
1 parent 837b500 commit 580b09a
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 16 deletions.
4 changes: 4 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ New features
2.4.0 (upcoming)
==================

Enhancements
------------
- In the theoretical VPF calculation (``theory.vpf``), the total volume of the random spheres can now exceed the volume of the sample [#238]

Bug fixes
---------
- Fix Python reference leak to results struct [#229]
Expand Down
16 changes: 0 additions & 16 deletions Corrfunc/theory/vpf.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,22 +196,6 @@ def vpf(rmax, nbins, nspheres, numpN, seed,
msg = "Number of counts-in-cells wanted must be at least 1"
raise ValueError(msg)

if boxsize > 0.0:
volume = boxsize * boxsize * boxsize
else:
volume = (max(X) - min(X)) * \
(max(Y) - min(Y)) * \
(max(Z) - min(Z))

volume_sphere = 4. / 3. * pi * rmax * rmax * rmax
if nspheres * volume_sphere > volume:
msg = "There are not as many independent volumes in the "\
"requested particle distribution. Num. spheres = {0} "\
"rmax = {1} => effective volume = {2}.\nVolume of particles ="\
"{3}. Reduce rmax or Nspheres"\
.format(nspheres, rmax, nspheres * volume_sphere, volume)
raise ValueError(msg)

# Ensure all input arrays are native endian
X, Y, Z = [convert_to_native_endian(arr, warn=True)
for arr in [X, Y, Z]]
Expand Down

0 comments on commit 580b09a

Please sign in to comment.