use_limits problems again

[woutdenolf]

I'm not using the GUI and want to understand the use_limits, xmin and xmax behavior for batch fitting McaAdvancedFitBatch, fast fitting FastXRFLinearFit and single spectrum fitting ClassMcaTheory.

Sorry but my blood boils every time I fall over this. I should calm down and actually understand the logic here.

So the single spectrum fitting ClassMcaTheory parses configuration (use_limits, xmin and xmax) and setData function parameters (xmin and xmax) like this in the setData function

pymca/src/PyMca5/PyMcaPhysics/xrf/ClassMcaTheory.py

Lines 1117 to 1139 in a072333

	xmin = self.config['fit']['xmin']
	if not self.config['fit']['use_limit']:
	if 'xmin' in kw:
	xmin=kw['xmin']
	if xmin is not None:
	self.config['fit']['xmin'] = xmin
	else:
	xmin=min(self.xdata)
	elif len(self.xdata):
	xmin=min(self.xdata)
	xmax = self.config['fit']['xmax']
	if not self.config['fit']['use_limit']:
	if 'xmax' in kw:
	xmax=kw['xmax']
	if xmax is not None:
	self.config['fit']['xmax'] = xmax
	else:
	xmax=max(self.xdata)
	elif len(self.xdata):
	xmax=max(self.xdata)
	self.lastxmin = xmin
	self.lastxmax = xmax

and apply it like this

pymca/src/PyMca5/PyMcaPhysics/xrf/ClassMcaTheory.py

Lines 1148 to 1155 in a072333

	#take the data between limits
	i1=numpy.nonzero((self.xdata >=xmin) & (self.xdata<=xmax))[0]
	self.xdata=numpy.take(self.xdata,i1)
	n=len(self.xdata)
	#Calculate the background just of the regions gives better results
	#self.zz=SpecfitFuns.subac(self.ydata,1.000,20000)
	#self.zz =numpy.take(self.zz,i1)
	self.ydata=numpy.take(self.ydata,i1)

So the behavior as I think I understand it:

• the configuration has xmin, xmax and use_limits.
• when use_limits is True we use the xmin and xmax values from the configuration for fitting and ignore the function arguments.
• when use_limits is False we use xmin and xmax from the function arguments for fitting and ignore the values from the configuration.

So configuration has priority when use_limits=True and function arguments has priority when use_limits=False.

This is confusing when I just look at the API of ClassMcaTheory.setData. This behavior exists for the GUI (see questions later).

McaAdvancedFitBatch behavior

Both in the new McaAdvancedFitBatch (with HDF5 output support) and LegacyMcaAdvancedFitBatch (no HDF5 output support) we force use_limit and completely ignore what was in the configuration:

pymca/src/PyMca5/PyMcaPhysics/xrf/McaAdvancedFitBatch.py

Line 757 in a072333

self.mcafit.config['fit']['use_limit'] = 1 # TODO: why???

pymca/src/PyMca5/PyMcaPhysics/xrf/LegacyMcaAdvancedFitBatch.py

Line 565 in a072333

self.mcafit.config['fit']['use_limit'] = 1

When using the McaAdvancedFitBatch I don't think you can pass xmin and xmax as arguments. So the result is: we always use xmin and xmax from the configuration no matter what.

ℹ️ I can solve the problem by patching the configuration

   batch = McaAdvancedFitBatch(...)

   if not configuration["fit"]["use_limit"]:
       # WARNING: McaAdvancedFitBatch ignores `use_limit=False`
       configuration["fit"]["xmin"] = 0
       configuration["fit"]["xmax"] = xrf_spectra.shape[1]

   batch.mcafit.configure(configuration)

FastXRFLinearFit behavior

We do not do such a thing in FastXRFLinearFit and LegacyFastXRFLinearFit but we have this

pymca/src/PyMca5/PyMcaPhysics/xrf/FastXRFLinearFit.py

Lines 133 to 138 in a072333

	if xmin is None:
	xmin = config['fit']['xmin']
	if xmax is None:
	xmax = config['fit']['xmax']
	dtypeCalculcation = self._fitDtypeCalculation(data)
	self._mcaTheory.setData(x=x, y=yref, xmin=xmin, xmax=xmax)

pymca/src/PyMca5/PyMcaPhysics/xrf/LegacyFastXRFLinearFit.py

Lines 117 to 121 in a072333

	if xmin is None:
	xmin = config['fit']['xmin']
	if xmax is None:
	xmax = config['fit']['xmax']
	toReconfigure = False

pymca/src/PyMca5/PyMcaPhysics/xrf/LegacyFastXRFLinearFit.py

Line 250 in a072333

self._mcaTheory.setData(x=x, y=firstSpectrum, xmin=xmin, xmax=xmax)

So when the xmin and xmax arguments are not provided to FastXRFLinearFit.fitMultipleSpectra we replace them by the configuration values and pass it to ClassMcaTheory.setData. So the result is: we always use xmin and xmax from the configuration (regardless of the value of use_limits) unless we provided them as function arguments to FastXRFLinearFit.fitMultipleSpectra.

ℹ️ I can solve the problem by calling fastFit.fitMultipleSpectra in this way

   fastFit = FastXRFLinearFit()

   if configuration["fit"]["use_limit"]:
       xmin = None
       xmax = None
   else:
       # WARNING: FastXRFLinearFit ignores `use_limit=False`
       xmin = 0
       xmax = xrf_spectra.shape[1]

  outbuffer = fastFit.fitMultipleSpectra(..., xmin=xmin, xmax=xmax)

Questions

1. So McaAdvancedFitBatch and FastXRFLinearFit ignores use_limits=False (they achieve this in a different way). I believe this was done because use_limits is only supposed to affect the GUI (see next question). Is that correct?
2. The priority of xmin and xmax configuration values vs. ClassMcaTheory.setData function arguments changes depending on whether use_limits is True or False. This had something to do with the GUI. When use_limits=True we ignore the zoom level of the spectrum plot and when use_limits=False the x limits are based on the zoom level of the spectrum plot. Something like that?

Once I understand this I'd like to have this documented somewhere.

[vasole]

I guess we'll have to meet but the original logic is (was?) the following:

In ClassMcaTheory, the priority should be given to the input arguments xmin and xmax if different from None

If they are None, then:

• if use_limits is True, then the fit configuration values prevail
• if use_limits is False, then the min and max values of x prevail

Please remember than nobody warrants xmin and xmax are channel numbers. Instead of 0 and shape[1] in your example, the correct values should be min(x) and max(x) (or x[0] and x[-1] if data are assumed to be sorted). They can be Energy values. Some users have data exported in energy and counts.

I hope that once that logic is understood, an implementation satisfying you can follow.

[vasole]

The fact all the different batch modules make sure the values from the configurations are taken is to make sure what the user set interactively via the GUI is what he will get when performing a batch.

If you are running things from outside the supplied modules (i.e. on your own workflows) all what you should need is to pass your own xmin and xmax values to have full control irrespective of the use_limits setting.

[vasole]

I agree that it is NOT what is implemented at this stage inside ClassMcaTheory.setData because if use_limits is True, it takes precedence over the user supplied xmin and xmax values and that is not correct.

[vasole]

In short, the correct (and I am pretty sure first implemented) logic is that if xmin and xmax are different from None should take priority over anything. Other ways why the user would bother setting them...

[vasole]

2. The priority of xmin and xmax configuration values vs. ClassMcaTheory.setData function arguments changes depending on whether use_limits is True or False. This had something to do with the GUI. When use_limits=True we ignore the zoom level of the spectrum plot and when use_limits=False the x limits are based on the zoom level of the spectrum plot. Something like that?

Yes, it was to make sure the user could decide if setting the fitting region via the zoom or not.

However, xmin and xmax should always have priority if different from None. To ignore them if different from None is to be considered a bug.

[vasole]

Today I would rise an exception if only one of xmin and xmax is None.

[woutdenolf]

In short, the correct (and I am pretty sure first implemented) logic is that if xmin and xmax are different from None should take priority over anything.

That would be indeed the most logical behavior. However in the current implementation of ClassMcaTheory.setData we see that if use_limit=True the xmin and xmax function arguments are ignored:

def setData(self,*var,**kw):
    xmin = self.config['fit']['xmin'] 
    if not self.config['fit']['use_limit']:
        ...  # look at kw and take it when not None

The blame shows that this code has not changed for 13 years. So perhaps there is another route than the ClassMcaTheory.setData function arguments that affects x limits but I don't see it.

[woutdenolf]

This commit from 2006 changed the logic: c713a49

[woutdenolf]

After discussion in person we decided that is a bug. I'll make a PR for this.