LBL-EESA / fastkde

Other
50 stars 10 forks source link

Array slicing error with numpy 1.23.5 #16

Closed jpobrien499 closed 1 year ago

jpobrien499 commented 1 year ago

This error is associated with running fastKDE.conditional(y,x) (as in the github example usage), with Python 3.8 and Numpy 1.23.5. Here is the error message:

1130     #Estimate the conditional
-> 1131     cpdf = _pdf.estimateConditionals(cvarInds,array(fullVarList),peakFrac = peakFrac)
   1132 
   1133     #Return the conditional and the axes

~/opt/anaconda3/envs/climate/lib/python3.8/site-packages/fastkde/fastKDE.py in estimateConditionals(self, variables, data, peakFrac, reApplyFilter)
    773       for i in leftSideVariableIndices:
    774           cslice[i] = slice(None,None,None)
--> 775       dxProd = dxProd[cslice[::-1]]
    776 
    777       normFactor = ma.masked_equal(sum(conditionalPDF*dxProd,axis=tuple(sumAxes)),0.0)

IndexError: only integers, slices (`:`), ellipsis (`...`), numpy.newaxis (`None`) and integer or boolean arrays are valid indices

It appears that indexing dxProd with cslice does not work in this version of numpy. I haven't tested it extensively, but replacing lines 773-775 with dxProd = expand_dims(dxProd,-1) seems to solve the issue.

kevindarby commented 1 year ago

I also get this in numpy 1.24.3

I don't really understand what the cslice is trying to do

Debugging the example in the readme, cslice = [slice(None, None, None), None]

@jpobrien499 does expand_dims work in all cases?

taobrienlbl commented 1 year ago

Thanks @jpobrien499 for reporting this, and thanks @kevindarby for confirming!

The idea there is to manipulate dxProd such that dxProd broadcasts correctly in dxProd*conditionalPDF (which happens a couple lines below 775). I think that @jpobrien499 's solution would work for 2D PDFs, but probably not higher rank.

@kevindarby - FYI, I won't have a chance to dig in to fastKDE in depth until Aug 11 (I have time set aside then). Until then, I just want to verify: when you say

Debugging the example in the readme, cslice = [slice(None, None, None), None]

do you mean that the README example triggers this error in numpy 1.24.3 ? Strange if so, since the IndexError seems to indicate that slice and None objects should both be valid. I wonder if the error is related to cslice being a list; if so, perhaps simply converting cslice from a list to a tuple might fix the issue (e.g., adding cslice = tuple(cslice)?

kevindarby commented 1 year ago

Hi @taobrienlbl - the readme example does indeed trigger this error in np 1.24.3, however

cslice = tuple(cslice)

fixed it!

taobrienlbl commented 1 year ago

Thanks @kevindarby for testing that: glad it seems to be a simple fix!

jpobrien499 commented 1 year ago

@kevindarby and @taobrienlbl Its been a while since I looked at this, but Im almost positive I tested expand_dims using both 2D and 3D cases and got the expected results for both. Regardless, glad this issue is closed either way!