ndtatbristol / arim

A Python library for array imaging in ultrasonic testing.
MIT License
23 stars 13 forks source link

ufunc unable to safely cast on Linux #17

Closed mgchandler closed 6 months ago

mgchandler commented 6 months ago

Running a multifrequency immersion simulation on both Windows and Linux machines, the Windows one runs correctly but Linux results in the following error:

TypeError: ufunc '_model_amplitudes_with_scat_matrix' not supported for the input types, and the inputs could not be safely coerced to any supported types according to the casting rule ''safe''

I've done a little digging around, it looks as though it's coming from the @numba.guvectorize decorator for the _model_amplitudes_with_scat_matrix function in model.py. It takes (among other things) the tx and rx list as arguments, which are ndarrays of type int64. The signature of the decorator currently specifies that tx and rx should be int32s, and presumably is throwing the error because it can't cast between these types. A quick fix is to change / add to the signature allowing tx and rx to be int64 as well, so it reads:

@numba.guvectorize(
    [
         "void(int32[:], int32[:], complex128[:,:], complex128[:], complex128[:], float64[:], float64[:], float64[:], complex128[:])",
         "void(int64[:], int64[:], complex128[:,:], complex128[:], complex128[:], float64[:], float64[:], float64[:], complex128[:])",
    ],
    "(n),(n),(s,s),(e),(e),(e),(e),()->(n)",
    nopython=True,
    target="parallel",
)
def _model_amplitudes_with_scat_matrix( ...

I don't 100% know why it's happening as I'm working with a 64-element array so I would have thought the cast is valid. Might be something on my end in my setup where the Linux install is less flexible than the Windows one, or possibly a result from when we changed the default types from the numpy ones to the python ones. However as it's a single addition that resolves it, it might be worth adding.

nbud commented 6 months ago

Hi Matt, thank you for the report.

I vaguely recall seeing this sort of errors when I first tested arim on Linux. It boils down to Linux and Windows using respectively int64 and int32 as a default integer type, and this trickles down to numpy and ultimately arim (relevant stackoverflow).

Your fix seems very reasonable, can you please create a pull request? If you add a unit test calling the code in question, that'll be even better, and the CI system on GitHub will automatically test it on both Linux and on Windows for you.

mgchandler commented 6 months ago

I've just noticed that I'm working in my branch contiguous-geometry which is outdated, but it looks like you've already addressed this in a4ef274. I should probably spend the time getting them back in sync again.