add_parameter() with DelegateParameter have inconsistent defaults for bind_to_instrument

[thangleiter]

Using the preferred pattern param = inst.add_parameter(...) together with param_cls=DelegateParameter defaults to bind_to_instrument=True, whereas DelegateParameter defaults to False. The latter's behavior is explicitly pointed out in the docs (which also makes sense to me).

However, forcing bind_to_instrument=False raises a QCoDeSDeprecationWarning.

MWE:

>>> from qcodes.instrument import Instrument
>>> from qcodes.parameters import DelegateParameter
>>> inst = Instrument('foo')
>>> param = inst.add_parameter('bar')
>>> delegate = inst.add_parameter('baz', DelegateParameter, source=param)
>>> inst.parameters
{'IDN': <qcodes.parameters.parameter.Parameter: IDN at 1349665492432>,
 'bar': <qcodes.parameters.parameter.Parameter: bar at 1349689473456>,
 'baz': <qcodes.parameters.delegate_parameter.DelegateParameter: baz at 1349691204784>}
>>> inst.remove_parameter('baz')
>>> delegate = inst.add_parameter('baz', DelegateParameter, source=param, bind_to_instrument=False)
2024-12-12 14:22:37,344 ¦ py.warnings ¦ WARNING ¦ warnings ¦ _showwarnmsg ¦ 112 ¦ ...\src\qcodes\instrument\instrument_base.py:184: QCoDeSDeprecationWarning: Parameter baz did not correctly register itself on instrument foo. Please check that `instrument` argument is passed from <class 'qcodes.parameters.delegate_parameter.DelegateParameter'> all the way to `ParameterBase`. This will be an error in the future.
  warnings.warn(

At the very least, the inconsistency should be resolved, however, I'd petition for allowing DelegateParameters not bound to instruments.

[jenshnielsen]

@thangleiter There is a logical error in add_parameter if bind_to_instrument is False. I am however curious of you use case for using add_parameter if you don't want to bind to the instrument? Why not simply create the Parameter with DelegateParameter("name", ...) for that use case? The only thing add_parameter really does is to create the parameter with the supplied args and then registers it on the instrument.

[thangleiter]

Hi @jenshnielsen, in the end it's not a crucial issue. It just came up as I was updating the TimeTagger driver to use the preferred pattern for adding parameters of using add_parameter and assigning the return value. That driver has some hacky helper parameters that don't need to be snapshotted or anything:
https://github.com/QCoDeS/Qcodes_contrib_drivers/blob/438417c01137b5a87adc0c5b67680e8fa09703f5/src/qcodes_contrib_drivers/drivers/SwabianInstruments/Swabian_Instruments_Time_Tagger.py#L297-L305

Obvioiusly, I could just not use add_parameter, but I think it's ill-advised to proliferate both the param = Parameter(...) and param = self.add_parameter(...) patterns now that there's finally a solution that combines the two patterns that existed alongside previously.

[jenshnielsen]

@thangleiter Thanks for the explanation.

If you want to avoid snapshotting I would recommend passing snapshot_exclude=True to add_parameter

Honestly, I never planned for the use case where someone would do `self.foo = self.add_parameter(..., bind_to_instrument=False) and the intension was that if a parameter was assigned as an attribute, it should also be bound to the instrument (which basically amount to adding it to the parameters dict.

I would be tempted to suggest that:
We fix the bug such that add_parameter with bind_to_instrument=False works correctly but make that behaviour warn and recomed not doing that highlighting snapshot_exclude as an alternative. Would that work for you?

[thangleiter]

That should work, yes.