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

Split up notebook check #892

Merged
merged 15 commits into from
Jan 31, 2024
Merged

Split up notebook check #892

merged 15 commits into from
Jan 31, 2024

Conversation

bharat-thotakura
Copy link
Contributor

@bharat-thotakura bharat-thotakura commented Jan 30, 2024

Interim test for #889

(Also re-runs some notebooks for updated target outputs and adds the available=True flag in _HPCA_Tutorial.ipynb)

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@bharat-thotakura bharat-thotakura changed the title [TESTING] Split notebook check Split up notebook check Jan 30, 2024
@bharat-thotakura
Copy link
Contributor Author

Reminder: #752

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@bharat-thotakura bharat-thotakura marked this pull request as draft January 31, 2024 00:03
bharat-thotakura and others added 6 commits January 30, 2024 18:03
Co-authored-by: richrines1 <85512171+richrines1@users.noreply.github.com>
Co-authored-by: richrines1 <85512171+richrines1@users.noreply.github.com>
Co-authored-by: richrines1 <85512171+richrines1@users.noreply.github.com>
Co-authored-by: richrines1 <85512171+richrines1@users.noreply.github.com>
Co-authored-by: richrines1 <85512171+richrines1@users.noreply.github.com>
Co-authored-by: richrines1 <85512171+richrines1@users.noreply.github.com>
Copy link
Contributor

@richrines1 richrines1 left a comment

Choose a reason for hiding this comment

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

this lgtm! when you're ready to merge we can switch the github rules to the new set of checks

thanks for updating this :)

@bharat-thotakura
Copy link
Contributor Author

bharat-thotakura commented Jan 31, 2024

this lgtm! when you're ready to merge we can switch the github rules to the new set of checks

thanks for updating this :)

Thanks! I'm good to merge this but also opened #893 as a brief solution (and also an attempt to begin the qss and css standardization moving forward) to drop the 'Notebook check for other docs' now rather than later -- if #893 looks good. Otherwise, we can keep the current update as is.

@bharat-thotakura bharat-thotakura marked this pull request as ready for review January 31, 2024 16:53
bharat-thotakura added a commit that referenced this pull request Jan 31, 2024
Updates some of the notebooks (and possibly dependent files) to have
clearer `qss` and `css` variants.

Motivated by
#892 (comment),
this PR should ideally remove the necessity of the 'Notebook check for
other docs' in #892 for now (see
#892 (comment))
@bharat-thotakura
Copy link
Contributor Author

Update: With #893 merged, this PR has been updated to only have the 'Notebook check for qiskit/cirq-superstaq'

cc: @vtomole @richrines1

@richrines1
Copy link
Contributor

updated the requirements, so should be good to merge!

given that the checks still takes a while there might be an argument for splitting "supermarq" off into its own category down the line...

@bharat-thotakura
Copy link
Contributor Author

^ I'm hoping the resolution to #772 would void the need for a further update since 'Notebook check for cirq-superstaq' finishes much faster and the cause seems to be the call to .backends()

@richrines1
Copy link
Contributor

^ I'm hoping the resolution to #772 would void the need for a further update since 'Notebook check for cirq-superstaq' finishes much faster and the cause seems to be the call to .backends()

ahh yep that makes sense

@bharat-thotakura
Copy link
Contributor Author

Made some interim changes with 1f953b0 that should hopefully cut down the qss notebook check time.

@bharat-thotakura bharat-thotakura merged commit 91d947f into main Jan 31, 2024
17 checks passed
@bharat-thotakura bharat-thotakura deleted the ci_notebook_update branch January 31, 2024 21:50
@bharat-thotakura bharat-thotakura linked an issue Feb 1, 2024 that may be closed by this pull request
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.

notebook check is very slow
2 participants