jjhelmus / nmrglue

A module for working with NMR data in Python
BSD 3-Clause "New" or "Revised" License
211 stars 86 forks source link

Correct frequency scale values from unit_conversion class #53

Closed kangaroocry closed 7 years ago

kangaroocry commented 8 years ago

In the previous way, all spectral scales had their first point at the very edge of the spectral window instead of half point distance away. This led the whole spectra be shifted a half point distance. For example, the first point, the middle between first and last point as well as the last point were at car/obs - delta*size/2, car/obs - delta/2 and car/obs + delta*(size/2 - 1). Now these positions are symmetric at car/obs - delta*(size - 1)/2, car/obs and car/obs + delta*(size - 1)/2. Moreover, now the middle of the spectrum is independent from the size of the data array. I think this is the usual way to distribute the data points over the spectral window and is also consistent with the method to create a unit conversion object from a frequency scale (see line 384). I hope I made no mistakes in my considerations.

kangaroocry commented 7 years ago

Sorry, maybe I was a bit quick with all the changes, I have to think about it a bit more and check whether everything is consistent and makes sense. I close the pull request for now. Sorry.

jjhelmus commented 7 years ago

@kangaroocry No worries, the unit conversion and PPM scales are hard to get right.

kangaroocry commented 7 years ago

I was confused by the issue of the shifted Bruker scale and was quite confident to have found the origin in the unit conversion class. Anyway, beside of that Bruker problem, I think there is still a half point offset for odd array sizes because the behavior of np.fft.fftshift is not correctly taken into account in that case. The first point of the initial FFT output should always represent the car frequency. For even size this works out together with fftshift. But for odd size fftshift puts this first point right to the middle of the scale (index (size-1)/2) while the unit_conversion class calculates all the values half a delta off: the first scale value is calculated relative to the car frequency with the expression self._size / 2. which is not an integer but an integer+0.5. Simply replacing self._size / 2. by (self._size//2) would put the car frequency to the correct index (size-1)/2. Am I correct?

jjhelmus commented 7 years ago

@kangaroocry You are probably correct on this issue but I'm far enough removed from NMR at this point that I don't know the answers to these types of questions any more. Why don't we keep the discussion on this topic to a single thread, perhaps #54. Hopefully some other NMR experts can chime in and offer help.