qiskit-experiments icon indicating copy to clipboard operation
qiskit-experiments copied to clipboard

Add **experiment_option parameter to experiment classes

Open conradhaupt opened this issue 2 years ago • 2 comments

Summary

The signatures of the __init__ functions for experiment classes are inconsistent between different experiments. For example, this can be seen when comparing the order of optional parameters in Rabi and CrossResonanceHamiltonian. Some optional parameters are experiment options, which have defaults defined in _default_experiment_options. This PR proposes a new structure for the __init__ functions for experiment classes.

Details and comments

Having an inconsistent order in the signatures means that the two classes are instantiated differently when keywords are not used for all parameters. The most common optional parameter is backend, which is used in the example below.

## Rabi
# We can provide `backend` as a positional argument only if we provide `amplitudes` as well.
Rabi(0,myschedule,amplitudes,backend)
# If we don't provide `amplitudes`, then we need to provide `backend` as a keyword argument.
Rabi(0,myschedule,backend=backend)

## CrossResonanceHamiltonian
# As `backend` is the first optional parameter in `__init__`, we don't need to provide a keyword to provide it.
CrossResonanceHamiltonian(0,flat_top_widths,backend)

Furthermore, some optional parameters are experiment options, such as amplitudes for Rabi. The proposed structure for experiment classes' __init__ functions is as follows:

  • qubit or qubits parameters come first in the signature (other than self).
  • Required parameters follow qubit. Examples include gate and schedule. These can be required experiment options.
  • The first optional parameter is backend.
  • Any other optional parameters that are not experiment options (i.e., defined in _default_experiment_options) follow backend.
  • Optional parameters that are experiment options are captured by a kwargs parameter **experiment_options. As experiment options should already be documented in _default_experiment_options, it does not make sense to repeat this information in __init__; other than to refer the reader to the _default_experiment_options documentation.

More on **experiment_options

The experiment_options parameter captures all keyword arguments that aren't explicitly defined in the __init__ signature. This means that a user can accidentally provide a parameter that is not used by the class. The dictionary experiment_options is passed to BaseExperiment.__init__ through super().__init__ and then passed to self.set_experiment_options by BaseExperiment. This means that all experiment subclasses must pass **experiment_options on to their call to super().__init__. The function set_experiment_options will throw an error if an unknown parameter keyword is encountered, which is useful to handle parameters not used by the class.

Changes to BaseExperiment to handle **experiment_options are introduced in this PR. To demonstrate how this change looks in experiment classes, QuantumVolume and CrossResonanceHamiltonian have been updated to use **experiment_options and its integration with BaseExperiment.

Edit [2022.08.08]: This PR currently contains changes to demonstrate the proposed solution and not all changes to implement the proposed interface to all experiment classes. This was not emphasized in the previous description of the PR.

cr_gate in CrossResonanceHamiltonian is a required experiment option. The updated experiment adds this argument to **experiment_options before passing it to super().__init__.

The recommended way to include **experiment_options is as follows:

class MyExperiment(BaseExperiment):
	
	def __init__(
		self,
		qubit : int,
		backend : Optional[Backend] = None,
		**experiment_options,
	):
    """ Create a MyExperiment instance.

    Args:
        qubit: ...
        backend: ...
        experiment_options: A catch-all for any experiment options. See
            :py:meth:`_default_experiment_options` for valid parameters.
    """
		super().__init__(
			[qubit],
			backend=backend,
			**experiment_options,
		)
		
	@classmethod
	def _default_experiment_options(cls):
		"""Default experiment options.
		
		Experiment Options:
			option1: Experiment option 1.
		"""
		...

conradhaupt avatar Jul 08 '22 11:07 conradhaupt

Thank you for the review, @nkanazawa1989 and @chriseclectic. I have implemented your feedback on the docstrings, which are much better now.

The current PR is only a proposal for a new interface and not yet a full refactoring of all classes. A few classes were modified to illustrate the proposed interface. This was not properly emphasized in the original PR description, but I have now added a note about this. If the proposed/modified interface is approved, I will update the remaining classes to conform to the agreed-upon style.

There are multiple options for handling **experiment_options in classes. The proposed solution (Sol. 1) has subclasses pass the experiment options' kwargs to the superclass. This continues up to BaseExperiment so that

  1. superclasses can validate and process experiment options if necessary,
  2. subclasses are not responsible for experiment options that are owned by a superclass,
  3. and subclasses do not need to call self.set_experiment_options - only respect the interface of their superclass (i.e., __init__).

An alternative (Sol. 2), as suggested by @chriseclectic, is to enforce that each experiment class calls self.set_experiment_options(**experiment_options) somewhere in __init__. This alternative bypasses any validation or processing super().__init__ may carry out on these experiment options but keeps the original interface for BaseExperiment. Options can be validated using Options.set_validator, but then we cannot validate options in __init__ as the experiment options may not be passed to the superclass. In my brief investigation of existing experiment classes, validation of experiment options in __init__ functions is limited to ensuring the correct type and whether an optional parameter is defined; something which can be done with the Options class.

A third solution (Sol. 3) is to explicitly define all experiment options, and other __init__ parameters, in the __init__ signatures for each experiment class. This means that

  1. it is more difficult to maintain consistent ordering of __init__ parameters between experiment classes and thus a consistent interface,
  2. subclasses have to include superclass __init__ parameters in their signature even if not used by the subclass,
  3. and dealing with a single kwargs variable (**experiment_options) can be cleaner than multiple explicitly defined kwargs.

A fourth alternative is to remove all optional experiment options from __init__ and require users to set them using experiment.set_experiment_options().

I have no preference between Sol. 1 and Sol. 2, but I do think a kwargs parameter is needed: i.e., I do not think Sol. 3 is a good solution. Experiment options are already defined and documentated in the class attribute experiment_options and internal class function _default_experiment_options. If it would make it easier to evaluate any solution, I can refactor more experiments so that more examples are available for the discussion.

conradhaupt avatar Aug 08 '22 12:08 conradhaupt

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jul 18 '23 13:07 CLAassistant