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 option to control figure generation in composite experiments #1240

Merged

Conversation

coruscating
Copy link
Collaborator

@coruscating coruscating commented Jul 28, 2023

Summary

Closes #1239. An init parameter has been added to composite analysis, generate_figures, to control figure generation. By default, generate_figures is always, meaning figures will always be generated. If generate_figures is set to selective, then only figures for analysis results of bad quality will be generated. If generate_figures is set to never, then figures will never be generated. This behavior can still be overridden for individual analyses by setting the analysis option plot, which is now unset by default instead of True.

Details and comments

  • I didn't end up implementing the proposal discussed in the issue where all figures would still be generated in selective mode if there are 10 or fewer figures. If this is preferable, we can add the behavior, but it seems a bit messy to keep a counter of all component experiments and how many figures each generates.
  • Added how to access child analysis classes in the Getting Started tutorial.

Timing benchmark

I ran a parallel T1 experiment over 127 qubits then timed only the analysis.

  • always: ~120 seconds
  • selective: ~100 seconds (generated 60/127 figures)
  • never: ~15 seconds

@@ -338,7 +337,7 @@ def _evaluate_quality(
Returns:
String that represents fit result quality. Usually "good" or "bad".
"""
if fit_data.reduced_chisq < 3.0:
if 0 < fit_data.reduced_chisq < 3.0:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This chisq > 0 condition has been added to quality=good because of edge cases where zero chisq is a sign that something is wrong (in this case, running RB with only one sample per length)
Pasted image 20230728110246

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the explanation. This change itself makes sense to me. (I'm not sure we can really determine if a fit is good or bad based on a single value, though, but I know it's another issue.)

@coruscating coruscating force-pushed the selective-figure-generation branch from b8defd2 to b3997cf Compare September 6, 2023 15:54
@coruscating coruscating added this to the Release 0.6 milestone Sep 7, 2023
Copy link
Contributor

@itoko itoko left a comment

Choose a reason for hiding this comment

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

Thank you, @coruscating. I think generate_figures is a useful option for CompositeAnalysis.
I have two high level questions.

  1. Should we expose generate_figures option in Experiment classes?
  2. Do we have any means to generate some figures (without rerunning analyses) after running analyses with generate_figures=="never"?

(1) I think an Experiment class should not depend on an Analysis class as possible (an Analysis class may depend on Experiment classes). Basically, I don't want the changes in Analysis class affect Experiment classes. In this case, asking users to set generate_figures via analysis option makes sense to me. What do you think?

(2) I guess no but I know adding such a feature is beyond the scope of this PR. I hope the on-going refactoring on AnalysisResult makes the future addition of the feature much easier.

@itoko itoko added the Changelog: New Feature Include in the "Added" section of the changelog label Sep 7, 2023
@coruscating
Copy link
Collaborator Author

coruscating commented Sep 7, 2023

@itoko Thanks for your review!

(1) I was thinking it would be convenient for users to specify generate_figures in the same interface as flatten_results, which is also an option set upon composite experiment instantiation that only affects the analysis and experiment data classes. I agree it would make more sense for both of these options to be analysis options, but changing the behavior of flatten_results would be a breaking API change so I'd rather keep it as is until we do the refactor. What do you think? I can also move generate_figures to the analysis class and keep flatten_results where it is now.

(2) Good question. There's no easy way to do that now since you have to give the plotter the series data manually. I'll write an issue to track this feature.

@itoko
Copy link
Contributor

itoko commented Sep 8, 2023

@coruscating thank you for the quick responses.

Regarding (1), your suggestion (move generate_figures to the analysis class and keep flatten_results where it is now to avoid a breaking API change within this PR) makes sense to me.

@coruscating
Copy link
Collaborator Author

@itoko I've updated generate_figures to be only a CompositeAnalysis option. I'm not sure why the lint test is failing—it's succeeding locally and the GitHub error message is on a file I didn't change.

Copy link
Contributor

@itoko itoko left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@coruscating coruscating added this pull request to the merge queue Oct 13, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 13, 2023
@coruscating coruscating added this pull request to the merge queue Oct 16, 2023
Merged via the queue into qiskit-community:main with commit 53b3033 Oct 16, 2023
10 checks passed
@coruscating coruscating deleted the selective-figure-generation branch October 21, 2023 01:35
nkanazawa1989 pushed a commit to nkanazawa1989/qiskit-experiments that referenced this pull request Jan 10, 2024
…kit-community#1240)

### Summary

Closes qiskit-community#1239. An init parameter has been added to composite analysis,
`generate_figures`, to control figure generation. By default,
``generate_figures`` is ``always``, meaning figures will always be
generated. If ``generate_figures`` is set to ``selective``, then only
figures for analysis results of bad quality will be generated. If
``generate_figures`` is set to ``never``, then figures will never be
generated. This behavior can still be overridden for individual analyses
by setting the analysis option ``plot``, which is now unset by default
instead of `True`.

### Details and comments
- I didn't end up implementing the proposal discussed in the issue where
all figures would still be generated in `selective` mode if there are 10
or fewer figures. If this is preferable, we can add the behavior, but it
seems a bit messy to keep a counter of all component experiments and how
many figures each generates.
- Added how to access child analysis classes in the Getting Started
tutorial.

### Timing benchmark
I ran a parallel T1 experiment over 127 qubits then timed only the
analysis.
- ``always``: ~120 seconds
- ``selective``: ~100 seconds (generated 60/127 figures)
- ``never``: ~15 seconds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make figure generation optional
2 participants