xraypy / xraylarch

Larch: Applications and Python Library for Data Analysis of X-ray Absorption Spectroscopy (XAS, XANES, XAFS, EXAFS), X-ray Fluorescence (XRF) Spectroscopy and Imaging, and more.
https://xraypy.github.io/xraylarch
Other
127 stars 62 forks source link

[xas_viewer] energy_shift #414

Closed maurov closed 1 year ago

maurov commented 1 year ago

@newville this is a follow-up of #357 . I have tried to fix it by myself, but I got lost in the Wx GUI stuff. If you could help with this soon, it would be great. The users at the beamline still find difficult to adjust the energy shifts with Larch. As a consequence, they go back using Athena...

The following are checked on the latest master version (xraylarch-0.9.65.post133+g5c834772)

  1. bug in copying energy reference groups
    • load two data groups (group1, group2), plus an energy reference one (group_eref)
    • select group1 and group2
    • click on group1 and set group_eref as energy reference
    • click copy in the energy reference
    • click on group2 -> BUG the energy reference is not group_eref (expected) but group2
  2. energy_shift is not kept
    • click on group1
    • put 1 in "energy shift from original" and press enter (just in case)
    • click on group2
    • click back on group1 -> BUG energy shift from original is 0
  3. energy_shift is not applied
    • click on group1
    • right click to copy group1 into group1_1
    • select group1 and group1_1
    • plot selected groups in dmu/dE -> are identical
    • click on group1_1 and put 1 into energy shift from original
    • replot selected groups -> BUG the two groups are not shifted
  4. applying energy_shift to reference does not propagate to groups linked to it
    • click on group1, put energy_shift to 0 and select group_eref as energy reference
    • click on group_eref and add 1 to energy_shift
    • click group1 -> BUG energy_shift is still at 0

The workaround to fix bugs 2 and 3 and get the expected behavior is to deselect plus select again the "auto?" next to E0 selection. I do not understand why this solves bug 2. For bug 3, we may apply directly the energy_shift value also to E0, or even better, E0 should simply refer to group.energy not to group.energy_orig.

Please, just to clarify my understanding of the energy_shift behavior in the GUI, could you check that the following is correct?

I hope this is clear, if not, please, let me know.

newville commented 1 year ago

@maurov Thanks - I'll look into each of those.

And, yes, the intention is that energy_orig is as read, and that energy = energy_orig + energy_shift is then used throughout, unless explicitly changing the energy shift.

newville commented 1 year ago

Hi Mauro,

Sorry for the delay on this.

On Thu, Nov 3, 2022 at 9:02 AM Mauro Rovezzi @.***> wrote:

@newville https://github.com/newville this is a follow-up of #357 https://github.com/xraypy/xraylarch/issues/357 . I have tried to fix it by myself, but I got lost in the Wx GUI stuff. If you could help with this soon, it would be great. The users at the beamline still find difficult to adjust the energy shifts with Larch. As a consequence, they go back using Athena...

The following are checked on the latest master version ( xraylarch-0.9.65.post133+g5c834772)

  1. bug in copying energy reference groups
    • load two data groups (group1, group2), plus an energy reference one (group_eref)
    • select group1 and group2
    • click on group1 and set group_eref as energy reference
    • click copy in the energy reference
    • click on group2 -> BUG the energy reference is not group_eref (expected) but group2

This works for me.

1.

  1. energy_shift is not kept
    • click on group1
    • put 1 in "energy shift from original" and press enter (just in case)
    • click on group2
    • click back on group1 -> BUG energy shift from original is 0

This also works for me.

1.

  1. energy_shift is not applied
    • click on group1
    • right click to copy group1 into group1_1
    • select group1 and group1_1
    • plot selected groups in dmu/dE -> are identical
    • click on group1_1 and put 1 into energy shift from original
    • replot selected groups -> BUG the two groups are not shifted

I do see this. And I have a fix for it, or at least think I have!

1.

  1. applying energy_shift to reference does not propagate to groups linked to it
    • click on group1, put energy_shift to 0 and select group_eref as energy reference
    • click on group_eref and add 1 to energy_shift
    • click group1 -> BUG energy_shift is still at 0

I think this is the same bug as the one above.

1.

The workaround to fix bugs 2 and 3 and get the expected behavior is to deselect plus select again the "auto?" next to E0 selection. I do not understand why this solves bug 2. For bug 3, we may apply directly the energy_shift value also to E0, or even better, E0 should simply refer to group.energy not to group.energy_orig.

I think the better workaround is to select the individual groups to force them to be reprocessed.

Please, just to clarify my understanding of the energy_shift behavior in the GUI, could you check that the following is correct?

  • read a group (g=Group()) -> g.energy = g.energy_orig + energy_shift, energy_shift = 0
  • when setting energy_shift to a value: g.energy = g.energy_orig + energy_shift
  • everything else (E0, plots, exports, whatever) uses g.energy
  • when read energy reference group (g_ref = Group()) -> g.energy_ref = g_ref or g.energy_ref = g_ref.name

I hope this is clear, if not, please, let me know.

Yes, I think that is all correct. To be clear the reference name is the "displayed File / Scan name", not the group name -- (it can have spaces, periods, etc). --Matt

SiebevanderVeer commented 1 year ago

Dear Mauro and Matt,

I'm not sure if I understand the above correctly, but I think that bug#3 reported above is now overcompensated. If I apply energy calibration to a spectrum and then copy the calibrated spectrum, the energy shift is applied an additional time to the copy. If I then try to correct the shift in the copy, by setting it back to 0 manually, both of the spectra get shifted back by the energy shift, resulting in undoing the calibration of the original spectrum. This is an issue when I wish to de energy calibration followed by deglitching, or compensation for overabsorption.

Best,

Siebe

maurov commented 1 year ago

@SiebevanderVeer thanks for reporting this misbehavior. I do think we still have bugs regarding the energy shift in the main GUI, plus the automatic energy shift/calibration widget, but it is not easy to debug them. By the way, which Larch version are you using?

@mretegan I think you had a look to this, do you have any comments to share?

SiebevanderVeer commented 1 year ago

@maurov No problem. The version I'm using is 0.9.66

newville commented 1 year ago

@SiebevanderVeer @maurov I don't disagree that there could be bugs in the energy alignment process with XAS Viewer, but I think this is working in a way that made sense to me.

I put the data sets I used at https://millenia.cars.aps.anl.gov/xraylarch/downloads/Ni2O3_energyshifts.zip

This has 3 real Ni K edge scans of Ni2O3, with a Ni foil reference channel, which are from a pretty old data collection: decent data, and a monochromator that does vary a bit. There is also a script that averages the data from scans 2 and 3 and applies a 4 eV shift -- that's just to make it so that there is not an energy shift that will give a "perfect fit".

If I read in scan 001 (so truly a different scan) and the "shifted spectra", with the energy references, I get the data session at https://millenia.cars.aps.anl.gov/xraylarch/downloads/Ni2O3_energyshifts.larix

If I then go to the "Calibrate Energy" dialog, I can select Ni2O3_shifted_ref.xdi and align that to Ni203_rt_01_ref.xdi. That gives a shift of -3.7 eV (or +3.7 eV if you go the other way).

If I then use "Select Groups with shared reference" and "Apply to selected groups" that appears to work correctly for me: the ~3.7 eV shift is written into the "Energy Shift" box for both "Ni2O3_shifte_ref" spectra, and they appear to be now be well-aligned with "Ni2O3_rt_01" spectra.

I do see a problem with "Save As New Group" that I think is consistent with Mauro's report: basically, this was making a new group that was shifted, but also keeping the original group as the energy reference channel. This is now fixed, so that "Save As New Group" creates a new group with the energy actually shifted (by the correct amount) and then it becomes "self referenced" and its energy shift set to 0, while still being aligned.

In general, I think "Save/Overwrite" should probably just be dropped from all the dialogs. That is, creating a new group and the user deleting the old group should probably be preferred over a single "overwrite in place".

Pushing this to "master development" branch, and hoping to release 0.9.67 relatively soon.

SiebevanderVeer commented 1 year ago

@newville, Thank you for the examples. Perhaps I have not followed the correct steps, but I do not end up with aligned data. I opened all four spectra simultaneously, which assigned spectrum 1 as the energy reference. Then when I align the shifted spectrum to spectrum 1, they indeed overlap very nicely after a shift of -3.61 has been applied. But for me when I then select all groups with a shared reference and apply the shift I basically get the orginal situation, i.e. small differences between 1-3 and the shifted spectrum misaligned by a few eV, but then with a -3.61 shift applied. This result makes sense to me and does not surprise me, but since you mention you end up with aligned spectra after doing this, this makes me curious if we are seeing different results?

maurov commented 1 year ago

@newville I am looking again to the issue of the energy shift, using your Ni2O3 dataset. Here some comments (using latest master version on a Debian WSL).

First of all, there is a bug when loading multiple files with energy reference channel. The groups get created correctly, but the link between the data an the energy reference is wrong, that is, all groups get the Energy Reference to Ni2O3_rt_01.xdi_ref, while they should be linked to their own energy reference groups (e.g. Ni2O3_rt_02.xdi with Ni2O3_rt_02.xdi_ref, Ni2O3_rt_02.xdi_ref with itself, and so on)

image

I then have to correct this manually.

This said, let me explain our workflow, because I have the feeling we do not follow the same procedure for correcting the energy alignment of the spectra. Let's take as example here Ni2O3_rt_01.xdi and Ni2O3_shifted.xdi.

Globally, this procedure works (with some glitches on Linux machines when clicking from one group to another). Personally, I would keep in the Calibrate / Align Energy panel only two buttons: 1) apply the calculated shift to the group that is being calibrated, 2) save as new group with new energy array and energy shift equal to 0. The current two buttons are misleading, even to myself.

Furthermore, I have observed another bug happening when merging two groups. Let's take our previous example where we have energy shifted one group (Ni2O3_shifted.xdi) on the first one (Ni2O3_rt_01.xdi) . If I merge the two groups by using the energy array of the one that has an energy shift (Ni2O3_shifted.xdi), then the value of shift is kept and thus applied twice.

image

To my opinion, the solution is that in case of merging multiple groups one should first apply the single shifts (in order to have all the spectra aligned) and then make a copy of the energy array with a shift 0 for the merged group.

newville commented 1 year ago

@maurov Thanks -- I'll investigate. That not reading of energy shifts per file does look like a bug.

I think that workflow is a common situation, but I might have to go through that in some detail.... but still aiming for Monday to fix this and multi-column read!

maurov commented 1 year ago

@newville thanks for having corrected the bug on correctly assigning the energy reference group when reading multiple column files.

I confirm that now applying the energy shifts works nicely.

The only last point I think is missing is the merging operation in case of selecting a group with an energy shift. I think the correct behavior is to first apply the energy shift and then merge. The new merged group should have always a new energy array and energy shift of 0.

newville commented 1 year ago

@maurov Yes, I agree that merging should merge energy-shifted data. I think that it should be doing that correctly, but I'll check that.

newville commented 1 year ago

@maurov I think that merging is working correctly, using energy-shifted data, and creating a group with no energy shift.

maurov commented 1 year ago

@newville thank you very much indeed for your work on this. Unfortunately, I still see the same mis-behavior at the merging two groups. Here what I do:

1) merge group1_no_shift and group2_shifted by using the energy array of group2_shifted 2) bug: the merged group has still the energy shift of group2_shifted -> should be 0

image

newville commented 1 year ago

@maurov -- will investigate. I'm guessing that if "match energy to" has an energy shift, that get copied to the new group. If so, it should be easy to fix....