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

Modification on E0 on EXAFS window is not reflected to the data [xas_viewer] #444

Closed Ameyanagi closed 8 months ago

Ameyanagi commented 1 year ago

Issue: E0 values are not saved to the Group when edited on the EXAFS tab of the xas_viewer. It can only be modified from the Normalization tab. Is this an expected feature? or is it an unexpected feature?

Initial attempts to solve this issue: (This did not work)

--- a/larch/wxxas/exafs_panel.py
+++ b/larch/wxxas/exafs_panel.py
@@ -123,7 +123,8 @@ class EXAFSPanel(TaskPanel):
                                    action=self.onProcess)
         wids['plot_win'].SetStringSelection('2')

-        opts = dict(digits=2, increment=0.1, min_val=0, action=self.onProcess)
+        opts = dict(digits=2, increment=0.1, min_val=0, action=self.onSet_XASE0Val)
+
         wids['e0'] = FloatSpin(panel, **opts)

         opts['max_val'] = 6
@@ -487,6 +488,12 @@ class EXAFSPanel(TaskPanel):
         self._set_frozen(evt.IsChecked())

+    def onSet_XASE0Val(self, event=None):
+        "handle setting e0"
+        self.update_config({'e0': self.wids['e0'].GetValue(),
+                            'auto_e0':False})
+        self.onProcess(event=event)
+
     def onRbkg(self, event=None):
         self.wids['fft_rmin'].SetValue(self.wids['rbkg'].GetValue())
         self.onProcess(event=event)

I was trying to fix it so it can be reflected in the group, but it seems it is not reflected in the original Group. There are also several places where similar things happen (sorry that I didn't remember where it was) and I want to be able to fix them when I find it. Can anyone help with this?

newville commented 1 year ago

@Ameyanagi I think E0 is correctly set from and with the EXAFS window. For example, if you change the value of E0, the background subtraction will be re-done and the new background will be shown. In EXAFSPanel.process() there is a section where it looks at whether the parameters for background subtraction have been changed and then runs the background subtraction commands if needed.

And inspecting the group's value for 'e0' in the Larch buffer shows the value from the EXAFS window.

Maybe you are seeing that the value for E0 on the Normalization tab is not also updated? That is an interesting question: should that value for E0 be pushed back to the normalization tab? I'm OK with Yes, that should be forced back, but I have a suspicion that some people are not going to love that idea ;).

Ameyanagi commented 1 year ago

@newville

Maybe you are seeing that the value for E0 on the Normalization tab is not also updated? That is an interesting question: should that value for E0 be pushed back to the normalization tab? I'm OK with Yes, that should be forced back, but I have a suspicion that some people are not going to love that idea ;).

This is what I am seeing. For example, if I change E0 in EXAFS window, the plot will be changed. (This works perfectly ok.) Then if I go to the Normalization tab and reopen the EXAFS tab, the E0 values are changed back to the E0 values defined in the Normalization tab. It feels like the data are kept on resetting and unstable.

I agree that pushing the E0 value back to the Normalization tab might not be a good solution. I personally don't want to do this.

Maybe I am feeling unstable when the values that I explicitly define is changed implicitly. I wish I had a better solution to this. Removing the E0 from the EXAFS tab (or just printing without letting the users change) will also solve this, but this would lead to deprecating some features.

newville commented 1 year ago

@Ameyanagi Yeah, I guess that going back and forth between Normalization and EXAFS background subtraction can lead to confusion about what E0 should be. Even though it is the same attribute, they are sort of used differently: "where to find the normalization step as post_edge - pre_edge" vs "where to define k=0".

We do want to be able to adjust the value from the EXAFS page: for some spectra, it is an important parameter to be able to change, and the value from Normalization might not be very good for background subtraction.

I suggest we add a button on the EXAFS tab next to the E0 value to allow the user to explicitly push that value back to the Normalization Page.

Ameyanagi commented 1 year ago

@newville I see. I also think the current settings are good in that case. I agree that adding a button in the EXAFS tab might be more explicit.

I can try adding it if you don't mind.

newville commented 1 year ago

@Ameyanagi Thanks. Yes, if you would like to add that, I would be in favor of that!

maurov commented 1 year ago

@newville @Ameyanagi I would like to give my opinion on this. I think we should distinguish the e0 used for normalization from the e0_k0 of EXAFS, not giving the possibility to affect the first one from the EXAFS panel.

In fact, e0 is used for energy calibration and for the pre/post-edge normalization, while e0_k0 for defining the k=0 point. I think it would be beneficial to store a separate variable e0_corr in eV as relative factor, that is, e0_k0 = e0 + e0_corr and put this variable in the EXAFS panel instead of e0.

What do you think?

newville commented 1 year ago

@maurov I would be OK with both:

1.  switch EXAFS background subtraction code to use "ek0", which would default to "e0".
2. have a button on the "EXAFS" tab of XAS Viewer to push "ek0" back to "e0".    Sometimes, you might want to do this....

I'll have to look into how hard step 1 will be ;).

Ameyanagi commented 1 year ago

@newville @maurov I think the given suggestions are a good choice. Since it might be a significant change, I will wait for @newville to look into it.

newville commented 1 year ago

@Ameyanagi this is now in 0.9.71!

Ameyanagi commented 1 year ago

@newville Thank you very much. I will check it.