manodeep / Corrfunc

⚡️⚡️⚡️Blazing fast correlation functions on the CPU.
https://corrfunc.readthedocs.io
MIT License
163 stars 50 forks source link

run time error when using theory.xi #206

Closed JohanComparat closed 4 years ago

JohanComparat commented 4 years ago

General information

Dear Manodeep Sinha,

I am using your very nice CorrFunc package (thank you for writing it) and got today a bug that apparently should be reported to you (the error message says so).

I paste below the error message. I also attach the script I wrote. To reproduce the bug, you need to download the simulation file I use :

http://www.mpe.mpg.de/~comparat/stuff/HMF_distinct_1.0.fit

Looking forward to your feedback on how to solve this issue.

Note that I installed CorrFunc with pip install and all the tests passed succesfully. The computation is done on a linux machine.

Thank you for your help,

Best wishes,

Johan

Issue description

run time error when using the theory.xi method

Expected behavior

no error

Actual behavior

run time error

What have you tried so far?

I copy-paste the example from the documentation (with actual simulated data in x,y,z), and it fails to run the correlation function

Minimal failing example

http://www.mpe.mpg.de/~comparat/stuff/002_2PCF.py http://www.mpe.mpg.de/~comparat/stuff/HMF_distinct_1.0.fit

Error message :

/home/comparat/miniconda3/envs/astroconda/lib/python3.7/site-packages/Corrfunc/utils.py:916: UserWarning: One or more input array has non-native endianness! A copy will be made with the correct endianness. warnings.warn("One or more input array has non-native endianness! A copy will"\ Error in file: ../../utils/gridlink_impl_double.c func: gridlink_double line: 188 with expression `X[i] >= xmin && X[i] <= xmax' x[0] = -0.000000 must be within [0.000000,4000.000000] Hopefully, input validation. Otherwise, bug in code: please email Manodeep Sinha manodeep@gmail.com

RuntimeError Traceback (most recent call last)

in 3 s1 = (x0ff>=x_min) & (x0ff14) & (X>0) & (X0) & (Y0) & (Z 5 wp_counts = wp(boxsize, pimax, nthreads, bins, x, y, z ) 6 xi_counts = xi(boxsize, nthreads, bins, x, y, z ) 7 ~/miniconda3/envs/astroconda/lib/python3.7/site-packages/Corrfunc/theory/wp.py in wp(boxsize, pimax, nthreads, binfile, X, Y, Z, weights, weight_type, verbose, output_rpavg, xbin_refine_factor, ybin_refine_factor, zbin_refine_factor, max_cells_per_dim, copy_particles, enable_min_sep_opt, c_api_timer, c_cell_timer, isa) 521 if extn_results is None: 522 msg = "RuntimeError occurred" --> 523 raise RuntimeError(msg) 524 else: 525 extn_results, api_time, cell_time = extn_results RuntimeError: RuntimeError occurred # rest of sample code goes here... ```
manodeep commented 4 years ago

@JohanComparat Thanks for the report. I am having trouble reproducing the bug since I get an error:

from astropy.io import fits
import numpy as np
p2s = 'HMF_distinct_1.0.fits'
hd = fits.open(p2s)
x0ff = hd[1].data['Xoff']/hd[1].data['rvir']
mass = np.log10(hd[1].data['Mvir'])
X, Y, Z = hd[1].data['x'], hd[1].data['y'], hd[1].data['z']

Each one of those keys (Xoff, rvir, Mvir, x, y, z) produce a KeyError. Is it possible that the datafiles are not the same? I will note that the script refers to a ".fits" file while the link is for a ".fit" file (missing terminating 's' )

I will also note that there is a warning produced about big-endian->little-endian conversion. As far as I know, fits stores data as big-endian and looks like the runtime environment is little-endian. One thing to try would be to convert the X, Y, Z to native-endian before starting any processing.

JohanComparat commented 4 years ago

Apologies, I uploaded the wrong file ... Here is the correct one : http://www.mpe.mpg.de/~comparat/stuff/distinct_1.0.fits

Indeed, there is something about the indianess, I will try to convert. Would you recommend a method to do this conversion ?

JohanComparat commented 4 years ago

I fixed the endianess by converting to a panda table as follows : t = Table.read(path_2_table) df = t.to_pandas() Now it works.

JohanComparat commented 4 years ago

Thanks !

manodeep commented 4 years ago

@JohanComparat Glad that you have a solution for your issue. Corrfunc is meant to work with non-native endian arrays; but we have a chequered history with non-native endian arrays -- e.g., issue #101 and #140, fixed in #142, re-introduced in #173, and re-fixed in #190. It seems that we may the bug once again.

I am re-opening this issue for further investigation. Please do leave the example fits file online to help us debug the root cause.

manodeep commented 4 years ago

@lgarrison What do you thinking about adding a dedicated test using non-native endian data?

lgarrison commented 4 years ago

That's probably a good idea, but I think the problem in this case is that we fixed it in #191 but haven't made a release since!

On Wed, Dec 18, 2019, 5:41 AM Manodeep Sinha notifications@github.com wrote:

@lgarrison https://github.com/lgarrison What do you thinking about adding a dedicated test using non-native endian data?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/manodeep/Corrfunc/issues/206?email_source=notifications&email_token=ABLA7S5S76J5YI3XBSL47JDQZH46XA5CNFSM4J3ZI5Y2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHFWIVI#issuecomment-566977621, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABLA7SZHYKWCGMCRTJ3R5T3QZH46XANCNFSM4J3ZI5YQ .

manodeep commented 4 years ago

@lgarrison Ohh good :)! Then I am happy to close this once we make the 2.3.2 release

manodeep commented 4 years ago

Corrfunc v2.3.2 is now out, both on PyPI and github. @JohanComparat If you can upgrade (e.g., pip install --upgrade Corrfunc to the latest version, then the original code should automatically work i.e., the conversion to native-endian should occur under the hood.

Closing this under the assumption that the issue is fixed. Please feel free to re-open if you still face the issue.