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

Many mentions to QASM should be changed to OpenQASM 2 #9351

Merged
merged 15 commits into from
Sep 29, 2023

Conversation

1ucian0
Copy link
Member

@1ucian0 1ucian0 commented Jan 8, 2023

Many parts of the documentation refer to "QASM" when they specifically talk about OpenQASM2. With many versions of QASM around and with OpenQASM 3.0 coming, I think it would help to be more explicit.

@1ucian0 1ucian0 requested review from a team, jyu00, manoelmarques and woodsp-ibm as code owners January 8, 2023 19:44
@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:

@mtreinish mtreinish added documentation Something is not clear or an error documentation Changelog: None Do not include in changelog labels Jan 9, 2023
@1ucian0 1ucian0 requested a review from ElePT as a code owner January 12, 2023 02:12
@coveralls
Copy link

coveralls commented Jan 12, 2023

Pull Request Test Coverage Report for Build 5879092683

  • 16 of 18 (88.89%) changed or added relevant lines in 10 files are covered.
  • 10 unchanged lines in 3 files lost coverage.
  • Overall coverage increased (+0.001%) to 87.26%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/circuit/quantumcircuit.py 0 1 0.0%
qiskit/qasm/qasm.py 0 1 0.0%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 93.76%
crates/qasm2/src/lex.rs 3 91.9%
crates/qasm2/src/parse.rs 6 97.13%
Totals Coverage Status
Change from base Build 5876541463: 0.001%
Covered Lines: 74303
Relevant Lines: 85151

💛 - Coveralls

@jlapeyre
Copy link
Contributor

jlapeyre commented Jan 12, 2023

In some places, we have "OpenQASM2", while in others "OpenQASM 2.0". Maybe I'm missing the context for why these are different. But we should probably use one or the other consistently.

The spec and documentation in the openqasm repo on github seem to use "OpenQASM X" or "OpenQASM X.Y", but not "OpenQASMX".

qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
qiskit/circuit/quantumcircuit.py Outdated Show resolved Hide resolved
qiskit/extensions/hamiltonian_gate.py Outdated Show resolved Hide resolved
qiskit/providers/models/backendconfiguration.py Outdated Show resolved Hide resolved
qiskit/qasm/node/binaryoperator.py Outdated Show resolved Hide resolved
qiskit/qobj/common.py Outdated Show resolved Hide resolved
@1ucian0
Copy link
Member Author

1ucian0 commented Jan 15, 2023

I didnt change it in the context of Qobj is because I was thinking at QASM qobj as a concept, as it is it's type in the Qobj structure.

@1ucian0 1ucian0 added this to the 1.0.0 milestone Sep 27, 2023
@coveralls
Copy link

coveralls commented Sep 27, 2023

Pull Request Test Coverage Report for Build 6351781302

  • 16 of 18 (88.89%) changed or added relevant lines in 10 files are covered.
  • 12 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.009%) to 87.005%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/circuit/quantumcircuit.py 0 1 0.0%
qiskit/qasm/qasm.py 0 1 0.0%
Files with Coverage Reduction New Missed Lines %
crates/qasm2/src/expr.rs 1 93.76%
crates/qasm2/src/lex.rs 5 91.41%
crates/qasm2/src/parse.rs 6 97.13%
Totals Coverage Status
Change from base Build 6349562467: -0.009%
Covered Lines: 74087
Relevant Lines: 85153

💛 - Coveralls

@jakelishman jakelishman changed the title Many mentions to QASM should be changed to OpenQASM2 Many mentions to QASM should be changed to OpenQASM 2 Sep 29, 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.

I've just fixed up the minor merge conflict and normalised the spacing of "OpenQASM[23]" to "OpenQASM [23]" - we were inconsistent before, and the space is more consistent with standard ways of writing versions in prose.

@jakelishman jakelishman added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Sep 29, 2023
@jakelishman jakelishman modified the milestones: 1.0.0, 0.25.2 Sep 29, 2023
@jakelishman
Copy link
Member

I was just planning to hit merge, but I see John's left a "request changes" review blocking that. @jlapeyre: I think all your comments are addressed within what we need to actually update - could you check again and approve / dismiss the block as appropriate?

@jlapeyre jlapeyre enabled auto-merge September 29, 2023 17:28
@jlapeyre jlapeyre added this pull request to the merge queue Sep 29, 2023
Merged via the queue into Qiskit:main with commit c6c9267 Sep 29, 2023
13 checks passed
mergify bot pushed a commit that referenced this pull request Sep 29, 2023
* Many mentions to QASM should be changed to OpenQASM2

* adjust tests

* another round

* suggestion in https://github.com/Qiskit/qiskit-terra/pull/9351/files#r1068676297

* https://github.com/Qiskit/qiskit-terra/pull/9351/files#r1068704441

* Normalise spacing of OpenQASM [23]

---------

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
(cherry picked from commit c6c9267)

# Conflicts:
#	qiskit/extensions/hamiltonian_gate.py
jakelishman pushed a commit that referenced this pull request Sep 29, 2023
* Many mentions to QASM should be changed to OpenQASM2

* adjust tests

* another round

* suggestion in https://github.com/Qiskit/qiskit-terra/pull/9351/files#r1068676297

* https://github.com/Qiskit/qiskit-terra/pull/9351/files#r1068704441

* Normalise spacing of OpenQASM [23]

---------

(cherry picked from commit c6c9267)
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
github-merge-queue bot pushed a commit that referenced this pull request Sep 29, 2023
* Many mentions to QASM should be changed to OpenQASM2

* adjust tests

* another round

* suggestion in https://github.com/Qiskit/qiskit-terra/pull/9351/files#r1068676297

* https://github.com/Qiskit/qiskit-terra/pull/9351/files#r1068704441

* Normalise spacing of OpenQASM [23]

---------

(cherry picked from commit c6c9267)
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>

Co-authored-by: Luciano Bello <bel@zurich.ibm.com>
ElePT pushed a commit to ElePT/qiskit-ibm-provider that referenced this pull request Oct 4, 2023
…9351)

* Many mentions to QASM should be changed to OpenQASM2

* adjust tests

* another round

* suggestion in https://github.com/Qiskit/qiskit-terra/pull/9351/files#r1068676297

* https://github.com/Qiskit/qiskit-terra/pull/9351/files#r1068704441

* Normalise spacing of OpenQASM [23]

---------

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
rupeshknn pushed a commit to rupeshknn/qiskit that referenced this pull request Oct 9, 2023
* Many mentions to QASM should be changed to OpenQASM2

* adjust tests

* another round

* suggestion in https://github.com/Qiskit/qiskit-terra/pull/9351/files#r1068676297

* https://github.com/Qiskit/qiskit-terra/pull/9351/files#r1068704441

* Normalise spacing of OpenQASM [23]

---------

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
ElePT pushed a commit to ElePT/qiskit-ibm-runtime that referenced this pull request Oct 10, 2023
…9351)

* Many mentions to QASM should be changed to OpenQASM2

* adjust tests

* another round

* suggestion in https://github.com/Qiskit/qiskit-terra/pull/9351/files#r1068676297

* https://github.com/Qiskit/qiskit-terra/pull/9351/files#r1068704441

* Normalise spacing of OpenQASM [23]

---------

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
ElePT pushed a commit to ElePT/qiskit-ibm-runtime that referenced this pull request Dec 8, 2023
…9351)

* Many mentions to QASM should be changed to OpenQASM2

* adjust tests

* another round

* suggestion in https://github.com/Qiskit/qiskit-terra/pull/9351/files#r1068676297

* https://github.com/Qiskit/qiskit-terra/pull/9351/files#r1068704441

* Normalise spacing of OpenQASM [23]

---------

Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
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 documentation Something is not clear or an error documentation stable backport potential The bug might be minimal and/or import enough to be port to stable
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants