Updated the calibration docs#1241
Conversation
|
|
|
@nitin-pandita Thanks for opening the PR—can you sign the CLA? |
Done |
eggerdj
left a comment
eggerdj
left a comment
There was a problem hiding this comment.
Thanks a lot for taking this on and improving the docs of Qiskit Experiments! The changes look mostly good to me up to a naming convention for calibration experiments.
| .. code-block:: python | ||
|
|
||
| RoughFrequencyCal(BaseCalibrationExperiment, QubitSpectroscopy) | ||
| class RoughFrequency(BaseCalibrationExperiment, QubitSpectroscopy): |
There was a problem hiding this comment.
This should have Cal at the end. We use this as convention to indicate that it is a calibration experiment.
| class RoughFrequency(BaseCalibrationExperiment, QubitSpectroscopy): | |
| class RoughFrequencyCal(BaseCalibrationExperiment, QubitSpectroscopy): |
| 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 |
There was a problem hiding this comment.
| This ensures that the `run` method of `RoughFrequency` will be the | |
| This ensures that the `run` method of `RoughFrequencyCal` will be the |
coruscating
left a comment
coruscating
left a comment
There was a problem hiding this comment.
Thanks for contributing. Note that Sphinx's rst notation is different from markup—one set of backticks makes the text italics instead of styled like code, so for most of the cases you want to use two sets of backticks. Please preview your docs by building locally or checking the artifacts produced during GitHub's CI runs.
| 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 |
There was a problem hiding this comment.
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
| 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 | ||
| an `auto_update` variable which, by default, is set to True. If this variable |
There was a problem hiding this comment.
| 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 |
| 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 | ||
| 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` |
There was a problem hiding this comment.
| 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` |
| #. `_get_schedules_from_options` | ||
| #. `_get_schedules_from_calibrations` | ||
| #. `_get_schedules_from_defaults` |
| 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 |
There was a problem hiding this comment.
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.
| 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 | ||
| :class:`~qiskit_experiments.calibration_management.update_library.BaseUpdater`. If no updater |
There was a problem hiding this comment.
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.
|
Closing this issue because Qiskit Experiments no longer supports Qiskit Pulse. Please reopen if you think this issue should not have been closed. |
5 participants
Summary
Please describe what this PR changes as concisely as possible. Link to the issue(s) that this addresses, if any.
Details and comments
Some details that should be in this section include:
Note that this entire PR description field will be used as the commit message upon merge, so please keep it updated along with the PR. Secondary discussions, such as intermediate testing and bug statuses that do not affect the final PR, should be in the PR comments.
PR checklist (delete when all criteria are met)
CONTRIBUTING.md.renoif this change needs to be documented in the release notes.