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

Add Experiment metadata #67

Merged
merged 10 commits into from
Jun 21, 2021

Conversation

chriseclectic
Copy link
Collaborator

Summary

Adds Experiment level metadata that is stored directly in ExperimentData without having to be extracted from a job result.

Experiment subclasses can override the BaseExperiment.metadata method to add additional experiment level metadata if required.

Details and comments

Adds metadata method to BaseExperiment which contains the experiment type, num qubits, physical qubits, and the option dicts for the exp (experiment, transpile, run, analysis). When ExperimentData is initialized from an Experiment it will store the Experiment.metadata() dict in the data object. When calling run or Analysis.run any runtime provided run or analysis options will also be stored in the ExperimentData metadata overriding the existing values.

Copy link
Contributor

@yaelbh yaelbh left a comment

Choose a reason for hiding this comment

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

We should rethink about circuit metadata. If it must contain all the information needed to generate the circuit, then experiment_options and num_qubits should be included (I haven't caught the difference between num_qubits and len(physical_qubits)). If not, then need to explain the necessity of the fields that are currently there.

qiskit_experiments/base_experiment.py Show resolved Hide resolved
qiskit_experiments/base_experiment.py Outdated Show resolved Hide resolved
@chriseclectic
Copy link
Collaborator Author

@yaelbh I added everything that is a @property of the BaseExperiment class. While num_qubits is not strictly necessary I find it quite convenient so you don't have to always do len(meta['physical_qubits']). Experiment type might not be necessary since I think it was stored separately as an input kwarg for the result DB ExperimentData class.

@yaelbh
Copy link
Contributor

yaelbh commented May 27, 2021

I'm reflecting about the circuit metadata this time, not the experiment metadata that you handle in this PR. The PR is a good opportunity to reconsider the circuit metadata, because information that until now was available only in the circuit metadata is available from now on also in the experiment metadata.

@chriseclectic chriseclectic force-pushed the exp-metadata branch 2 times, most recently from 90ea9df to b1d7066 Compare June 18, 2021 14:29
Copy link
Contributor

@eggerdj eggerdj left a comment

Choose a reason for hiding this comment

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

Overall this looks fine. My Major concern is the duplication of the MockJob with the existing TestJob. Do we need both?

test/mock_job.py Outdated Show resolved Hide resolved
qiskit_experiments/base_experiment.py Show resolved Hide resolved
qiskit_experiments/base_experiment.py Show resolved Hide resolved
Adds all the experiment options as a subdict in metadata keyed by the job id. This lets options for successive executions all get stored so that they can be retrieved for each run job.
* Make job_metadata a list so execution order of jobs is known
* Add validation when adding additional data to an existing ExperimentData object
This method should be overridden by experiment subclasses to add metadata rather than overriding the main method.
Copy link
Contributor

@eggerdj eggerdj left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Collaborator

@nkanazawa1989 nkanazawa1989 left a comment

Choose a reason for hiding this comment

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

Approved with future update for #120

@yaelbh
Copy link
Contributor

yaelbh commented Jun 21, 2021

Is it possible to postpone to after the release? Will there be hard requirements of modifications to existing experiments?

@nkanazawa1989
Copy link
Collaborator

You mean this PR or the issue? I think issue can be addressed in the next release (0.2). However it is preferable to address this before the release because we may deprecate one of two places storing analysis options (before developers start writing a code in different styles).

@chriseclectic chriseclectic merged commit a8eeb58 into Qiskit-Extensions:main Jun 21, 2021
@chriseclectic chriseclectic deleted the exp-metadata branch July 21, 2021 18:45
paco-ri pushed a commit to paco-ri/qiskit-experiments that referenced this pull request Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants