ignore xmin and xmax when use_limit is set in the configuration #1129
Conversation
woutdenolf
force-pushed
the
1128-fastxrflinearfit-ignores-use_limit
branch
from
35ddd86 to
f7ad9b6
Compare
| if xmin is None: | ||
| xmin = config['fit']['xmin'] | ||
| if xmax is None: | ||
| xmax = config['fit']['xmax'] |
There was a problem hiding this comment.
This essential forced use_limits=True regardless of where it is true or false in the config file.
There was a problem hiding this comment.
Because it is the desired behavior in the fast linear fit to prevent user mistakes.
If xmin and xmax are passed, then they are used. If not the fit configuration is used. It has been always the case.
There was a problem hiding this comment.
I would revert that modification. It can only be harmful. because many users do not set use_limits=True
You can see in the McaAdvancedFit that it is done internally by setting xmax and xmin to the fit configuration values.
| 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) | ||
| if self.config['fit']['use_limit']: | ||
| xmin = self.config['fit']['xmin'] | ||
| else: | ||
| xmin = kw.get('xmin') | ||
| if xmin is not None: | ||
| self.config['fit']['xmin'] = xmin | ||
| 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) | ||
| xmin = min(self.xdata) | ||
| else: | ||
| xmin = 0 | ||
|
|
||
| if self.config['fit']['use_limit']: | ||
| xmax = self.config['fit']['xmax'] | ||
| else: | ||
| xmax = kw.get('xmax') | ||
| if xmax is not None: | ||
| self.config['fit']['xmax'] = xmax | ||
| elif len(self.xdata): | ||
| xmax=max(self.xdata) | ||
| xmax = max(self.xdata) | ||
| else: | ||
| xmax = 0 |
There was a problem hiding this comment.
This code was very hard to understand so I modified it to make it more clear (at least imo).
There was a problem hiding this comment.
- use_limit=True -> use the config
- use_limit=False:
- function argument not None -> use function arguments
- function argument None :
- xdata not empty -> use first/last value
- xdata empty -> 0
There was a problem hiding this comment.
However, the fit configuration limits should not be changed. xmin and xmax is what is going to be used at the end.
Well, it seems it was the previous behavior when explicit values were passed.
|
I see a regression in fit quality due to this commit. Need to investigate what's going on ... |
|
Ok no, my mistake I was calculating a theoretical spectrum with use_limit=1 and then fitting it with use_limit=1 while I wanted to calculate with use_limit=0 and fit with use_limit=1. |
|
It seems it was the case before when explicit limits were passed. |
|
Then how about this # Force fit limits to be used, whether the user enabled it or not.
config['fit']['use_limit'] = 1instead of this if xmin is None:
xmin = config['fit']['xmin']
if xmax is None:
xmax = config['fit']['xmax'] |
|
If the user explicitly ask you to use different limits, then you have to use them. I cannot afford that by performing a fit, a parameter that only has sense in interactive mode gets changed. If you set it like that, the user will never be able to select the fitting region because the limits will always be hard-coded after selecting the first fitting region. The use_limit flag only has sense within the interactive use and it was explicitly requested to achieve that behavior: to give the user the possibility to fix or not the zoomed region to be fitted. It is the same as with the parameter to ignore calibration from input data:
|
Closes #1128