Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reader for Neaspec interferogram txt file #717

Closed
wants to merge 63 commits into from

Conversation

brnovasco
Copy link
Contributor

Reader for the NeaSpec multichannel interferogram file in the txt format. This reader performs the multiplication of respective amplitude and phase channels before creating the table, in a similar process as it is done in the equipment that generates the averaged data.

This procedure allow us to separate spectra data for individual runs so, relevant data can be selected and processed in the Orange workflows.

brnovasco and others added 30 commits November 16, 2022 16:27
… numpy.convolve. Needs routine to wrong input type.
… editor). Find y and calculate the angle on the widget is the best option? Can I make ManualTilt return the angle as metadata (together with the default data) so the widget in Manual Editor could display it?
…ontrol line is moved and draw new diagonal for better visualization.
…utomatic callback after the rerutn key was pressed in the previous element on the chain (gui.spin) affecting the desired changes on the widget. Setting it to false, created the expected behaviour.
Fixing callback for only updating the slider spin box for only updating the slider step.
@brnovasco brnovasco marked this pull request as draft May 7, 2024 12:55
brnovasco added 3 commits May 7, 2024 10:04
…lines of the output table, so the same settings can be used in the fft calculation for neaSPEC data.
… and reading the default value from the data attributes if it exists.
@brnovasco brnovasco marked this pull request as ready for review May 7, 2024 21:07

info = self.data.attributes
try:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additions to this widget should also be tested (make a test, that inputs some data without the scaling factor, check that it remains 1, add the scaling to data.attributes, input again, and confirm that it is set.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @markotoplak , I missed from the title this PR was also changing FFT. I would prefer to keep them separate in the future.

What is the physical origin of this scaling factor?

@borondics
Copy link
Member

borondics commented May 14, 2024 via email

@stuart-cls
Copy link
Member

@borondics I see. Generally I agree we do need an energy calibration tool, I have had to do this before for laser-based systems for example.

Why don't you apply the scaling factor to the dx value directly? dx_corrected = dx / scale gives the same output as this post-processing approach, without adding a ton of extra checkboxes and so on:

dx = 0.048967
scale = 0.979340
Nzff = 16
dx2 = dx / scale

freq = np.fft.rfftfreq(Nzff, dx)
scale_freq = freq * scale
freq2 = np.fft.rfftfreq(Nzff, dx2)
assert (freq2 == scale_freq).all()

There's a second problem with this implementation however: There appear to be multiple dx values inside the table. The OWFFT code is generating the dx from the .attributes info (as before) but there is also a "delta_x" column which appears to vary from one group of 01A->01P spectra to another but this is not accounted for in the calculation.

@borondics
Copy link
Member

You have a point Stuart...

However, the GUI features and the location of the multiplication are independent if we want to keep the possibility for manual input, which could be nice for systems where the scale factor can't be read.

Also, if we ever manage to implement the water vapor calibration via fitting, it would be probably better to do it in the frequency space to avoid continuous FFTs during the fitting process. Although this might not happen too many times and maybe not too costly either.

@stuart-cls
Copy link
Member

You can already incorporate a scaling factor in the GUI by changing the dx value manually. If that entry is locked out currently for Neaspec files (it might be), we should make the GUI less rigid. If calculating by hand is too tedious, we could put a handler in that can evaluate simple entries like "2123.4 / 2343" which then stores the correct dx as a setting.

I agree energy calibration is best done in frequency space.

@lucyanomacedo
Copy link

The dx feature added accounts for the regularly spaced datapoints for the interferogram that is needed for the FFT. At this point, it is not reasonable to have this dx multiplied by the scaling factor prior to the FFT, although too many dx are displayed in the input dataset for the FT indeed.
All the OnA and OnP will present the same dx. Each run has its own dx value, based on the encoder absolute x scan reading.
We could remove the box for manual scaling factor input and leave just the reader make the multiplication of the input values scaling factor extracted from the .txt reader.

@stuart-cls
Copy link
Member

Hi @lucyanomacedo

Each run has its own dx value, based on the encoder absolute x scan reading.

Is it possible/normal for multiple runs to be in a file/dataset (as they are in the test file?) If so the current code does not handle this condition.

At this point, it is not reasonable to have this dx multiplied by the scaling factor prior to the FFT

I'm afraid I don't follow your point here!

It seems to me that the error is in the dx value, and we should correct it there. This could be a good time to clean up the dx section: I'm not opposed to adding a scaling factor setting, as Feri mentioned there could be times users want to manually enter it. We could also drop the "HeNe" checkbox and expose the undersampling setting.

We could remove the box for manual scaling factor input and leave just the reader make the multiplication of the input values scaling factor extracted from the .txt reader.

I do prefer this solution, it hides instrumental corrections which cannot be meaningfully altered from the user. We could always add the manual scaling later if it turns out to be useful.

@brnovasco brnovasco marked this pull request as draft May 26, 2024 16:11
@markotoplak
Copy link
Collaborator

What is the state of this PR?

Regarding functionality:

  • We need a clear consensus as to what the FFT widget extension should be.

I do not know enough about what FFT additions we need, and thus, I leave it up to you guys. @lucyanomacedo, @stuart-cls, @brnovasco, @borondics. If the current solution already is the consensus you are looking for, just write a comment to confirm it.

From the maintainability point of view:

  • This PR should be rebased and commits not addressing the feature should be removed.
  • Some old code of NeaReader got written slightly differently (with different spacing and string characters) - we should avoid doing this for no reason, because then we lose track of changes. I can refactor these out.
  • There are no tests for the FFT widget addition.

I can work maintainability part. Therefore, @brnovasco, could you please tell me if you have time and will to do any of these so that I know how to proceed? I would suggest that you commit whatever you've done up till now, and then I could rebase this branch, and then we could continue with other stuff.

So, guys, let's finish this one soon! It's been standing here for long enough.

@stuart-cls
Copy link
Member

@markotoplak Thank you for the summary and for pushing things along! Feri and I are meeting today, we will discuss.

@stuart-cls
Copy link
Member

If I can summarize my understanding of the status of the FFT part (@borondics please correct me)

  • Current approach is best (correct known scaling error using information in the file)
  • If we have to add a new setting to manually enter this value, it should modify the dx (but maybe that could be a separate PR)
  • Outstanding question: is the case where different scaling factors are present in the same dataset (as in the example file) real, or invented? If it's real, the current approach does not work.

@raul-freitas
Copy link

Hello guys, I will try to help here as we did some research on the metadata content. Here is the current header of the interferograms files:

# www.neaspec.com
# Scan:	 	Fourier Scan
# Project:	 	user_Jun2024
# Description:	 	Au_ref5
# Date:	 	06/06/2024 11:16:43
# Scanner Center Position (X, Y):	[µm]	45.11	43.71	 
# Rotation:	[°]	0	 	 
# Scan Area (X, Y, Z):	[µm]	0.000	0.000	0.000
# Pixel Area (X, Y, Z):	[px]	1	1	1024
# Interferometer Center/Distance:	[µm]	715.000	489.670	 
# Averaging:	 	45	 	 
# Integration time:	[ms]	20	 	 
# Wavenumber Scaling:	 	0.979340	 	 
# Laser Source:	 	Synchrotron
# Detector:	 	R
# Target Wavelength:	[µm]		 	 
# Demodulation Mode:	 	Fourier
# Tip Frequency:	[Hz]	283,708.188	 	 
# Tip Amplitude:	[mV]	60.000	 	 
# Tapping Amplitude:	[nm]	85.484	 	 
# Modulation Frequency:	[Hz]	0.000	 	 
# Modulation Amplitude:	[mV]	0.000	 	 
# Modulation Offset:	[mV]	0.000	 	 
# Setpoint:	[%]	80.45	 	 
# Regulator (P, I, D):	 	2.077946	2.992700	1.000000
# Tip Potential:	[mV]	0.000	 	 
# M1A Scaling:	[nm/V]	0.362	 	 
# M1A Cantilever Factor:	 	1,000	 	 
# Q-Factor:	 	486.9	 	 
# Version:	 	2.1.11145.0

From the Neaspec team discussion, we know that the M columns in the interferograms file is the raw position sensor reading. Moreover, the metadata shows a line # Wavenumber Scaling: which should be applied to the data stepsize (dx). That said, we see 3 modifications to be implemented for us to read these raw interferograms files:

  1. The file reader widgets must read the ifg.txt files and re-organize the data to be compatible to the current FFT widget.
  2. As the raw data has no equidistant stepsize, we need to interpolate and resample it. For that, we will double check whether the current Interpolate widget can do it. If not, we will suggest an adaptation.
  3. For the Interferogram to Spectrum widget, we will include the Wavenumber Scaling factor, read from the header of this specific file, into the data stepsize calculation.

We may separate those modifications in different PRs as you prefer. For this current PR, we will move forward regarding the modification 1 (file reader widgets). Sounds good?

@stuart-cls
Copy link
Member

Ah, I missed that the dx is irregular. I think that Interpolate will work well for this. The rest sounds good to me.

If it is not realistic that different datasets (groups of rows) can have different scaling factors, it should be removed from the test file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants