Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated the calibration docs #1241

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -44,54 +44,53 @@ class BaseCalibrationExperiment(BaseExperiment, ABC):

This abstract class extends a characterization experiment by turning it into a
calibration experiment. Such experiments allow schedule management and updating of an
instance of :class:`.Calibrations`. Furthermore, calibration experiments also specify
an auto_update variable which, by default, is set to True. If this variable,
is True then the run method of the experiment will call :meth:`~.ExperimentData.block_for_results`
and update the calibrations instance once the backend has returned the data.
instance of :class:`~qiskit_experiments.calibration_management.Calibrations`. Furthermore, calibration experiments also specify
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is unneeded. The dot syntax in Sphinx links to the correct class with that name as long as it's unique. You can see for yourself in the current docs that Calibrations is already correctly linked: https://qiskit.org/ecosystem/experiments/dev/stubs/qiskit_experiments.calibration_management.BaseCalibrationExperiment.html#qiskit_experiments.calibration_management.BaseCalibrationExperiment

an `auto_update` variable which, by default, is set to True. If this variable
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
an `auto_update` variable which, by default, is set to True. If this variable
an ``auto_update`` variable which, by default, is set to True. If this variable

is True, then the `run` method of the experiment will call :meth:`~qiskit_experiments.base_experiment.ExperimentData.block_for_results`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
is True, then the `run` method of the experiment will call :meth:`~qiskit_experiments.base_experiment.ExperimentData.block_for_results`
is True, then the :class:`~.BaseCalibrationExperiment.run` method of the experiment will call :meth:`~.ExperimentData.block_for_results`

and update the `Calibrations` instance once the backend has returned the data.

This mixin class inherits from the :class:`.BaseExperiment` class since calibration
experiments by default call :meth:`~.ExperimentData.block_for_results`. This ensures that the next
This mixin class inherits from the :class:`~qiskit_experiments.base_experiment.BaseExperiment` class since calibration
experiments by default call :meth:`~qiskit_experiments.base_experiment.ExperimentData.block_for_results`. This ensures that the next
calibration experiment cannot proceed before the calibration parameters have been
updated. Developers that wish to create a calibration experiment must subclass this
base class and the characterization experiment. Therefore, developers that use this
mixin class must pay special attention to their class definition. Indeed, the first
class should be this mixin and the second class should be the characterization
experiment since the run method from the mixin must be used. For example, the rough
frequency calibration experiment is defined as
experiment since the `run` method from the mixin must be used. For example, the rough
frequency calibration experiment is defined as follows:

.. code-block:: python

RoughFrequencyCal(BaseCalibrationExperiment, QubitSpectroscopy)
class RoughFrequency(BaseCalibrationExperiment, QubitSpectroscopy):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have Cal at the end. We use this as convention to indicate that it is a calibration experiment.

Suggested change
class RoughFrequency(BaseCalibrationExperiment, QubitSpectroscopy):
class RoughFrequencyCal(BaseCalibrationExperiment, QubitSpectroscopy):

pass

This ensures that the ``run`` method of :class:`.RoughFrequencyCal` will be the
run method of the :class:`.BaseCalibrationExperiment` class. Furthermore, developers
must explicitly call the :meth:`__init__` methods of both parent classes.
This ensures that the `run` method of `RoughFrequency` will be the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This ensures that the `run` method of `RoughFrequency` will be the
This ensures that the `run` method of `RoughFrequencyCal` will be the

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure I will fix that

`run` method of the `BaseCalibrationExperiment` class. Furthermore, developers
must explicitly call the `__init__` methods of both parent classes.

Developers should strive to follow the convention that the first two arguments of
a calibration experiment are the qubit(s) and the :class:`.Calibrations` instance.
a calibration experiment are the qubit(s) and the :class:`~qiskit_experiments.calibration_management.Calibrations` instance.

If the experiment uses custom schedules, which is typically the case, then
developers may chose to use the :meth:`get_schedules` method when creating the
circuits for the experiment. If :meth:`get_schedules` is used then the developer
must override at least one of the following methods used by :meth:`get_schedules`
developers may choose to use the `get_schedules` method when creating the
circuits for the experiment. If `get_schedules` is used, then the developer
must override at least one of the following methods used by `get_schedules`
to set the schedules:

#. :meth:`_get_schedules_from_options`
#. `_get_schedules_from_options`
#. `_get_schedules_from_calibrations`
#. `_get_schedules_from_defaults`
Comment on lines +80 to +82
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These methods and BaseCalibrationExperiment.get_schedules() were removed in #547, therefore this section needs to be updated as I highlighted in the issue. @eggerdj what do you think the text here should be? Should these two paragraphs simply be removed?


#. :meth:`_get_schedules_from_calibrations`
These methods are called by `get_schedules`.

#. :meth:`_get_schedules_from_defaults`

These methods are called by :meth:`get_schedules`.

The :meth:`update_calibrations` method is responsible for updating the values of the parameters
stored in the instance of :class:`.Calibrations`. Here, :class:`BaseCalibrationExperiment`
provides a default update methodology that subclasses can override if a more elaborate behaviour
is needed. At the minimum the developer must set the variable :code:`_updater` which
should have an :code:`update` method and can be chosen from the library
:mod:`qiskit_experiments.calibration_management.update_library`. See also
:class:`qiskit_experiments.calibration_management.update_library.BaseUpdater`. If no updater
is specified the experiment will still run but no update of the calibrations will be performed.
The `update_calibrations` method is responsible for updating the values of the parameters
stored in the instance of :class:`~qiskit_experiments.calibration_management.Calibrations`. Here, `BaseCalibrationExperiment`
provides a default update methodology that subclasses can override if a more elaborate behavior
is needed. At the minimum, the developer must set the variable `_updater` which
should have an `update` method and can be chosen from the library
:mod:`~qiskit_experiments.calibration_management.update_library`. See also
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a module, just a file, so you need to update the language here. The Sphinx syntax in this paragraph also needs to be fixed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On it

:class:`~qiskit_experiments.calibration_management.update_library.BaseUpdater`. If no updater
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This link won't work because BaseUpdater isn't currently exposed in the API toctree. You need to add from .update_library import BaseUpdater to calibration_management/__init__.py.

is specified, the experiment will still run, but no update of the calibrations will be performed.
"""

def __init_subclass__(cls, **kwargs):
Expand Down