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

Fixing small bugs and improving output #19

Merged
merged 12 commits into from
Jan 8, 2025
Merged

Fixing small bugs and improving output #19

merged 12 commits into from
Jan 8, 2025

Conversation

rabrie
Copy link
Collaborator

@rabrie rabrie commented Dec 16, 2024

  • Fixed GHZ plot showing wrong qubit counts
  • Add GST config variable to pass to run_MGST(..., testing=) for plots
  • In mgst.algorithm at line 721 add success=1 so that correct message is printed if the threshold was not reached but the alg. converged (with model mismatch etc.)
  • Add note about the GST optional dependency in the installation guide somewhat prominently and also in the GST example notebooks.
  • mGST docstrings in low_level_jit are wrong: "derivative by dK" instead of "derivative of dK" and so on
  • Add gate labels qubit indices to GST gate labels in the compressive_gst.py
  • GST saves an empty "y" in init
  • Transpiled and untranspiled circuits were swapped in circuit class change for GST
  • GHZ Fidelity plot: Adding grid, better x-labels, backend and timestamp in title, consistent coloring
  • Added remaining GST print statements to log

@rabrie rabrie changed the title fixed GST issues except logging Fiying small GST bugs and improving output Dec 17, 2024
@rabrie rabrie changed the title Fiying small GST bugs and improving output Fiying small bugs and improving output Dec 20, 2024
@pedrofigro pedrofigro changed the title Fiying small bugs and improving output Fixing small bugs and improving output Dec 23, 2024
@rabrie rabrie marked this pull request as ready for review January 7, 2025 09:16
@rabrie rabrie requested a review from pedrofigro January 7, 2025 09:20
Copy link
Collaborator

@pedrofigro pedrofigro left a comment

Choose a reason for hiding this comment

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

I only noticed little issues in docstrings for the GST files (some parameters missing, for example), but this doesn't have to be resolved in this PR. I'll continue reviewing the checklist points.

README.md Outdated Show resolved Hide resolved
examples/example_gst.ipynb Outdated Show resolved Hide resolved
src/mGST/low_level_jit.py Outdated Show resolved Hide resolved
@pedrofigro
Copy link
Collaborator

The GHZ plot now shows the correct number of qubits! But the labels are funny, showing halves, for example. It'd be nicer to show labels only on the qubit counts of the experiment and a grid in the background. I also generally prefer seeing other information in the plot like backend name and time/date of the experiment.
image

rabrie and others added 3 commits January 7, 2025 14:14
Co-authored-by: Rakhim Davletkaliyev <rakhim.davletkaliyev@meetiqm.com>
Co-authored-by: Rakhim Davletkaliyev <rakhim.davletkaliyev@meetiqm.com>
Co-authored-by: Rakhim Davletkaliyev <rakhim.davletkaliyev@meetiqm.com>
src/mGST/algorithm.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@pedrofigro pedrofigro left a comment

Choose a reason for hiding this comment

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

Approving now, given that the issues with the new circuit container are critical; can address the smaller comments in a separate PR.

@rabrie
Copy link
Collaborator Author

rabrie commented Jan 8, 2025

The GHZ plot now shows the correct number of qubits! But the labels are funny, showing halves, for example. It'd be nicer to show labels only on the qubit counts of the experiment and a grid in the background. I also generally prefer seeing other information in the plot like backend name and time/date of the experiment.

This is now included and also the plot colors are adapted to be in line with RB plots.
Example:
image

Copy link
Collaborator

@pedrofigro pedrofigro left a comment

Choose a reason for hiding this comment

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

neat! I've seen the prints changed to loggings now too, things look good from my side

@rabrie rabrie merged commit 35deb81 into main Jan 8, 2025
2 checks passed
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.

3 participants