ldeo-glaciology / xapres

package for processing ApRES data using xarray
MIT License
3 stars 2 forks source link

padding/rolling chirps leaves two erroneous zeros #48

Closed jkingslake closed 1 month ago

jkingslake commented 2 months ago

When the chirps are padded and rolled (in one step) prior to performing the fft in ChirpObject.FormProfile, the indexing is slightly wrong and it does not replace two values in an array of zeros with data when they should be.

In the two lines starting here parts of an array of zeros are replaces with two parts of the chirp - the first half and the second half. The Indexing goes to -1, as follows

padchirp[0:math.floor(Nt/2)-1] = winchirp[math.floor(Nt/2):-1]
padchirp[-math.floor(Nt/2):-1] = winchirp[0:math.floor(Nt/2)-1] 

but because numpy's index excludes the final index, indexing till -1 is like saying you want to stop at the penultimate element.

b = np.arange(10)
print(b[0:2])

returns [0 1]

This didn't raise an error because it simply didn't replace the final value in padchirp (padchirp[-1]) and a value to the right of the data on the left (padchirp[math.floor(Nt/2)-1]).

A simple patch would be to replace these two lines with

padchirp[0:math.floor(Nt/2)] = winchirp[math.floor(Nt/2):]
padchirp[-math.floor(Nt/2):] = winchirp[0:math.floor(Nt/2)]

A better solution would be overhaul that section of the code and instead use np.pad and np.roll

s_windowed = s * np.blackman(N)
chirp_padded = np.pad(s_windowed, pad_width=int((N*pad-N)/2))
chirp_padded_rolled = np.roll(chirp_padded, shift = N)
jkingslake commented 2 months ago

cc @glugeorge

This errors has been in the code form the beginning, so all power profiles we have processed with this code were made with chirps containing two erroneous zeros. I don't know if this makes a significant difference, but it is something worth double checking I think.

jkingslake commented 1 month ago

PR #51 will close this.

jkingslake commented 1 month ago

closed with #51