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

Raise error eagerly on bad unitary synthesis method #11367

Merged
merged 6 commits into from
Dec 5, 2023

Conversation

rupeshknn
Copy link
Contributor

@rupeshknn rupeshknn commented Dec 4, 2023

Summary

generate_preset_pass_manager now raises an error the pass manager is created, not when it is run.

Fixes #11355

@qiskit-bot qiskit-bot added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Dec 4, 2023
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

@CLAassistant
Copy link

CLAassistant commented Dec 4, 2023

CLA assistant check
All committers have signed the CLA.

@coveralls
Copy link

coveralls commented Dec 4, 2023

Pull Request Test Coverage Report for Build 7095555132

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.03%) to 87.455%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 4 91.67%
Totals Coverage Status
Change from base Build 7078267006: 0.03%
Covered Lines: 59930
Relevant Lines: 68527

💛 - Coveralls

Copy link
Contributor

@kevinsung kevinsung left a comment

Choose a reason for hiding this comment

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

Thanks! Please add a test.

@rupeshknn
Copy link
Contributor Author

Do you have a recommendation for where the test should be located? I couldn't find test files (within synthesis) that check whether a particular error was raised.

Or I could add one where the pass manager is tested. https://github.com/Qiskit/qiskit/blob/main/test/python/transpiler/test_preset_passmanagers.py

@rupeshknn rupeshknn requested a review from kevinsung December 4, 2023 20:53
@rupeshknn
Copy link
Contributor Author

I've added the test to test/python/transpiler/test_stage_plugin.py:

kevinsung
kevinsung previously approved these changes Dec 4, 2023
Copy link
Contributor

@kevinsung kevinsung 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!

kevinsung
kevinsung previously approved these changes Dec 4, 2023
@jakelishman jakelishman changed the title fix issue #11355 Raise error eagerly on bad unitary synthesis method Dec 5, 2023
Copy link
Member

@jakelishman jakelishman 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!

@jakelishman jakelishman enabled auto-merge December 5, 2023 02:54
@jakelishman jakelishman added Changelog: Bugfix Include in the "Fixed" section of the changelog mod: transpiler Issues and PRs related to Transpiler labels Dec 5, 2023
@jakelishman jakelishman added this to the 1.0.0 milestone Dec 5, 2023
@jakelishman jakelishman added this pull request to the merge queue Dec 5, 2023
Merged via the queue into Qiskit:main with commit b1fc8e9 Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: Bugfix Include in the "Fixed" section of the changelog Community PR PRs from contributors that are not 'members' of the Qiskit repo mod: transpiler Issues and PRs related to Transpiler
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

specifying nonexistent synthesis plugin does not raise error
6 participants