Skip to content
Merged
Show file tree
Hide file tree
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
2 changes: 1 addition & 1 deletion qiskit_experiments/framework/base_experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ def run(

# Optionally run analysis
if analysis and experiment.analysis:
return self.analysis.run(experiment_data)
return experiment.analysis.run(experiment_data)
else:
return experiment_data

Expand Down
6 changes: 6 additions & 0 deletions qiskit_experiments/framework/composite/composite_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ def component_analysis(self, index=None) -> Union[BaseAnalysis, List[BaseAnalysi
return self._analyses
return self._analyses[index]

def copy(self):
ret = super().copy()
# Recursively copy analysis
ret._analyses = [analysis.copy() for analysis in ret._analyses]
return ret

def _run_analysis(self, experiment_data: ExperimentData):
# Return list of experiment data containers for each component experiment
# containing the marginalied data from the composite experiment
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,15 @@ def copy(self) -> "BaseExperiment":
ret = super().copy()
# Recursively call copy of component experiments
ret._experiments = [exp.copy() for exp in self._experiments]

# Check if the analysis in CompositeAnalysis was a reference to the
# original component experiment analyses and if so update the copies
# to preserve this relationship
if isinstance(self.analysis, CompositeAnalysis):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like Tphi experiment unintentionally breaks this relationship. This there any safer logic for developer to notify this? This is not the scope of this PR.
https://github.com/Qiskit/qiskit-experiments/blob/66002c536777f955b7349057c0f60924d98dc940/qiskit_experiments/library/characterization/tphi.py#L104-L108

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure if it was intended or not for Tphi, but if analysis was supposed to be a reference line 108 should be changed to be TphiAnalysis(analyses=[exp_t1.analysis, exp_t2.analysis])

for i, orig_exp in enumerate(self._experiments):
if orig_exp.analysis is self.analysis._analyses[i]:
# Update copies analysis with reference to experiment analysis
ret.analysis._analyses[i] = ret._experiments[i].analysis
return ret

def _set_backend(self, backend):
Expand Down
10 changes: 10 additions & 0 deletions releasenotes/notes/fix-composite-copy-a869e9773f6a4d48.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
fixes:
- |
Fix :meth:`.ParallelExperiment.copy` and :meth:`.BatchExperiment.copy`
so that the copies preserves any references between the original
component experiments analysis classes and the :class:`.CompositeAnalysis`
classes component analysis classes.
- |
Fixes :meth:`.CompositeAnalysis.copy` to recursively make a copy of the
component analysis classes.
29 changes: 29 additions & 0 deletions test/test_composite.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,35 @@ def test_composite_copy(self):
self.check_attributes(new_instance)
self.assertEqual(new_instance.parent_id, None)

def test_composite_copy_analysis_ref(self):
"""Test copy of composite expeirment preserves component analysis refs"""

class Analysis(FakeAnalysis):
"""Fake analysis class with options"""

@classmethod
def _default_options(cls):
opts = super()._default_options()
opts.option1 = None
opts.option2 = None
return opts

exp1 = FakeExperiment([0])
exp1.analysis = Analysis()
exp2 = FakeExperiment([1])
exp2.analysis = Analysis()

# Generate a copy
par_exp = ParallelExperiment([exp1, exp2]).copy()
comp_exp0 = par_exp.component_experiment(0)
comp_exp1 = par_exp.component_experiment(1)
comp_an0 = par_exp.analysis.component_analysis(0)
comp_an1 = par_exp.analysis.component_analysis(1)

# Check reference of analysis is preserved
self.assertTrue(comp_exp0.analysis is comp_an0)
self.assertTrue(comp_exp1.analysis is comp_an1)

def test_nested_composite(self):
"""
Test nested parallel experiments.
Expand Down