Unsetting the default expression of 'VoigtModel' and 'SkewedVoigtModel' does not work as expected

[Julian-Hochhaus]

First Time Issue Code

Yes, I read the instructions and I am sure this is a GitHub Issue

Hint: Potentially there are several different small bugs, if necessary, I will separate them into separate issues.

Description

According to the manual (see Notes of Parameter class), an expression is unset setting either par.set(expr='') or par.set(vary=True).

For the VoigtModel as well as for the SkewedVoigtModel, the gamma parameter is by default constrained to the sigma parameter (compare l.734 and l.1284 in models.py respectively).

To be able to independently vary sigma and gamma, I tried unsetting the constraint of the gamma parameter as described in the manual. However, the results for both commands ( par.set(expr='') and par.set(vary=True)) differ (BUG 1).

In addition, normally I would have expected that there should be no difference in the behavior of par.set(vary=True) and par.vary=True. Trying to unset the default constraint of the gamma parameter of VoigtModel and SkewedVoigtModel using the two different methods, however, resulted in different behavior (BUG 2). In the Minimal working example below I have only shown the details including the output and some comments for the VoigtModel, the behavior is the same for the SkewedVoigtModel, which could be verified by the code snippet at the end of the MWE.

But first, I will give some commented examples of the observed bugs:

Unsetting using vary=True

from lmfit.models import VoigtModel, SkewedVoigtModel
PeakModel = VoigtModel()
pars1 = PeakModel.make_params()
print('default:',pars1['gamma'])
pars1['gamma'].vary = True
print('unset',pars1['gamma'])

pars1 = PeakModel.make_params()
print('default:', pars1['gamma'])
pars1['gamma'].set(vary = True)
print('unset:',pars1['gamma'])

Output:

default: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
unset: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
default: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
unset: <Parameter 'gamma', value=1.0, bounds=[-inf:inf]>

Problem:

Behavior of par.set(vary=True) and par.vary=True is different, while par.set(vary=True) does indeed unset the expression, par.vary=True seems to do nothing

Unsetting using expr=''

from lmfit.models import VoigtModel, SkewedVoigtModel
PeakModel2 = VoigtModel()
pars2 = PeakModel2.make_params()
print('default:',pars2['gamma'])
pars2['gamma'].expr = ''
print('unset:',pars2['gamma'])

PeakModel2 = VoigtModel()
pars2 = PeakModel2.make_params()
print('default:',pars2['gamma'])
pars2['gamma'].set(expr='')
print('unset:',pars2['gamma'])

Output:

default: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
unset: <Parameter 'gamma', value=1.0 (fixed), bounds=[-inf:inf]>
default: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
unset: <Parameter 'gamma', value=1.0 (fixed), bounds=[-inf:inf]>

Problem: The behavior of par.set(expr='') and par.expr='' is the same, and both are unsetting the expr. However, in addition the command results in addition in setting the parameter fixed in both cases (BUG 3).

Complete MWE

from lmfit.models import VoigtModel, SkewedVoigtModel
PeakModel = VoigtModel()
pars1 = PeakModel.make_params()
print('default:',pars1['gamma'])
pars1['gamma'].vary = True
print('unset:',pars1['gamma'])
print('*****')
PeakModel2 = VoigtModel()
pars2 = PeakModel2.make_params()
print('default:',pars2['gamma'])
pars2['gamma'].expr = ''
print('unset:',pars2['gamma'])
print('*****')
PeakModel3 = SkewedVoigtModel()
pars3 = PeakModel3.make_params()
print('default:',pars3['gamma'])
pars3['gamma'].vary = True
print('unset:',pars3['gamma'])
print('*****')
PeakModel4 = SkewedVoigtModel()
pars4 = PeakModel4.make_params()
print('default:',pars4['gamma'])
pars4['gamma'].expr = ''
print('unset:',pars4['gamma'])
print('*****')
print('*****')
print('*****')

PeakModel = VoigtModel()
pars1 = PeakModel.make_params()
print('default:', pars1['gamma'])
pars1['gamma'].set(vary = True)
print('unset:',pars1['gamma'])
print('*****')
PeakModel2 = VoigtModel()
pars2 = PeakModel2.make_params()
print('default:',pars2['gamma'])
pars2['gamma'].set(expr='')
print('unset:',pars2['gamma'])
print('*****')
PeakModel3 = SkewedVoigtModel()
pars3 = PeakModel3.make_params()
print('default:',pars3['gamma'])
pars3['gamma'].set(vary = True)
print('unset:',pars3['gamma'])
print('*****')
PeakModel4 = SkewedVoigtModel()
pars4 = PeakModel4.make_params()
print('default:',pars4['gamma'])
pars4['gamma'].set(expr='')
print('unset:',pars4['gamma'])

Complete Output:

default: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
unset: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
*****
default: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
unset: <Parameter 'gamma', value=1.0 (fixed), bounds=[-inf:inf]>
*****
default: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
unset: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
*****
default: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
unset: <Parameter 'gamma', value=1.0 (fixed), bounds=[-inf:inf]>
*****
*****
*****
default: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
unset: <Parameter 'gamma', value=1.0, bounds=[-inf:inf]>
*****
default: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
unset: <Parameter 'gamma', value=1.0 (fixed), bounds=[-inf:inf]>
*****
default: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
unset: <Parameter 'gamma', value=1.0, bounds=[-inf:inf]>
*****
default: <Parameter 'gamma', value=1.0, bounds=[-inf:inf], expr='sigma'>
unset: <Parameter 'gamma', value=1.0 (fixed), bounds=[-inf:inf]>

Process finished with exit code 0

Version information

Python: 3.8.3 (default, Jul 2 2020, 16:21:59)
[GCC 7.3.0]
lmfit: 1.0.3.post46+g589c029, scipy: 1.5.0, numpy: 1.23.4,asteval: 0.9.26, uncertainties: 3.1.6

[newville]

@Julian-Hochhaus Thanks. I agree that is a bit of a wart.

From a purely API point-of-view, I think it is not unusual for "set attribute" to have a different result from "call a function with a keyword argument". The function is almost certainly going to do more work unless there is a sort-of-magic "setter method". It's OK to expect that we might use these for a lmfit.Parameter (currently "yes" for expr, "no" for vary), but I don't think that this is necessarily "normal".

For a parameter that is already constrained with an expression, doing

    param.expr = '' 

is sort of ambiguous -- it does not specify whether the value should be varied or not. I think the current behavior is defensible, though I can also see that making it variable would be defensible too. As it happens, I think that changing the implementation of this behavior is a bit subtle.

Doing

    param.vary = True

but not also un-setting the expression seems more problematic to me. There, the intent is clear, and the value of vary should override the value of expr. It also turns out that using a "setter" method for vary (we currently do not) would easily fix this, and in a way that does not break anything else.

[Julian-Hochhaus]

From a purely API point-of-view, I think it is not unusual for "set attribute" to have a different result from "call a function with a keyword argument". The function is almost certainly going to do more work unless there is a sort-of-magic "setter method". It's OK to expect that we might use these for a lmfit.Parameter (currently "yes" for expr, "no" for vary), but I don't think that this is necessarily "normal".

Indeed, I fully agree that the expected behavior of calling a function is different to setting single attributes . I have never thought about it enough.

In addition, I suggest pointing out in the documentation the different behavior of par.set(vary=True) and par.set(expr='') while unsetting expressions, otherwise remains confusing.

[newville]

With #860, doing param.vary = True will also set the expressions attribute to '' so that the parameter will be varied in the fit. I think that is the more important behavior to fix.

Setting param.expr to '' is a little less obvious about whether that parameter value and vary state.

[newville]

@Julian-Hochhaus I believe this is addressed in 1.2.0