Skip to content

Conversation

jakelishman
Copy link
Member

Summary

The newest control-flow QPY tests do not have hard-coded names on the outer circuits, which trips assertion errors if the library import causes any more circuits to be instantiated during build (since the generated names are circuit-count dependent).

Inner circuits do not need naming in the same way, since these are not used in the assertion; nested circuits' names would be tested separately. The control-flow builder interface, for example, does not assign these circuits names at all.

Summary

Details and comments

The newest control-flow QPY tests do not have hard-coded names on the
outer circuits, which trips assertion errors if the library import
causes any more circuits to be instantiated during build (since the
generated names are circuit-count dependent).

Inner circuits do not need naming in the same way, since these are not
used in the assertion; nested circuits' names would be tested
separately. The control-flow builder interface, for example, does not
assign these circuits names at all.
@jakelishman jakelishman added type: qa Issues and PRs that relate to testing and code quality stable backport potential The bug might be minimal and/or import enough to be port to stable Changelog: None Do not include in changelog labels Oct 13, 2023
@jakelishman jakelishman requested a review from a team as a code owner October 13, 2023 10:36
@qiskit-bot
Copy link
Collaborator

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

  • @Qiskit/terra-core
  • @mtreinish
  • @nkanazawa1989

@coveralls
Copy link

Pull Request Test Coverage Report for Build 6507182825

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 87.04%

Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/lex.rs 3 90.91%
Totals Coverage Status
Change from base Build 6502891788: 0.0%
Covered Lines: 74377
Relevant Lines: 85452

💛 - Coveralls

@mtreinish mtreinish enabled auto-merge October 13, 2023 11:21
@mtreinish mtreinish added this pull request to the merge queue Oct 13, 2023
Merged via the queue into Qiskit:main with commit a1927bd Oct 13, 2023
mergify bot pushed a commit that referenced this pull request Oct 13, 2023
The newest control-flow QPY tests do not have hard-coded names on the
outer circuits, which trips assertion errors if the library import
causes any more circuits to be instantiated during build (since the
generated names are circuit-count dependent).

Inner circuits do not need naming in the same way, since these are not
used in the assertion; nested circuits' names would be tested
separately. The control-flow builder interface, for example, does not
assign these circuits names at all.

(cherry picked from commit a1927bd)
github-merge-queue bot pushed a commit that referenced this pull request Oct 13, 2023
The newest control-flow QPY tests do not have hard-coded names on the
outer circuits, which trips assertion errors if the library import
causes any more circuits to be instantiated during build (since the
generated names are circuit-count dependent).

Inner circuits do not need naming in the same way, since these are not
used in the assertion; nested circuits' names would be tested
separately. The control-flow builder interface, for example, does not
assign these circuits names at all.

(cherry picked from commit a1927bd)

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
@jakelishman jakelishman deleted the fix-qpy-backwards-compat-names branch October 17, 2023 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog mod: qpy Related to QPY serialization stable backport potential The bug might be minimal and/or import enough to be port to stable type: qa Issues and PRs that relate to testing and code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants