RNO-G / mattak

RNO-G dataformats
1 stars 1 forks source link

type casting of waveforms in python API #33

Closed cozzyd closed 4 months ago

cozzyd commented 4 months ago

int16 numpy arrays can behave unexpectedly (e.g. easy to accidentally overflow) and in most cases, one will want to do something with the waveforms that results in them being converted to floatings anyway.

I attempted to add some control over the type returned by wfs( ) in the Python API by adding a wanted_type argument.

Right now, this defaults to float64, but perhaps it should be float32 (currently, the C++ calibrated waveforms are 64-bit floats, though perhaps they shouldn't be?). For time-domain stuff, float32 is probably fine, but when you do an FFT it will get converted to a float64 anyway, I think, so adding this extra step may not be worth the memory savings? Either way the user has control...

If the user passes None to the wanted_type, it will behave as before.

fschlueter commented 4 months ago

What is the advantage of astype over asarray? Maybe that we can set safe="safe"?

fschlueter commented 4 months ago

However, I have to say I am not a big fan of this. I think it makes the code just another step more complicated for a questionable use case (in my opinion).

cozzyd commented 4 months ago

astype vs. asarray probably both work fine here, though I don't know if asarray behaves the same with the frombuffer used in the PyROOT backend (i.e. elides the copy if the requested type is the same). You're right, astype with casting='safe', to prevent someone from e.g. going to int8 or an unsigned type.

Do you think it would be better to just always cast to float64 and not provide the option to not?

fschlueter commented 4 months ago

Yes I would always cast.

fschlueter commented 4 months ago

So we decided to always cast the output array to float. That was merged wit #24 .