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

update: restricting parameter names to not collide with ones we use for OpenQASM generation. #675

Merged
merged 14 commits into from
Aug 21, 2023

Conversation

krneta
Copy link
Contributor

@krneta krneta commented Aug 14, 2023

Issue #, if available:
#603

Description of changes:
When generating OpenQASM we decided to use 'b' and 'q' for 'bits' and 'qubits' variables, but this can collide with parameters chosen by customers. I've renamed these to __bits__ and __qubits__ and have changed our requirements for FreeParameter names so that we exclude names that start with double underlines. I've also updated the verification function for our FreeParameter names to be closer to the identifiers definition from OpenQASM to further reduce changes there will be transformation errors (some of which are mentioned in the ticket above).

This check is not exact, so it's possible that we'll allow for FreeParameter names that are not translatable to OpenQASM, since there's a trade-off between performance and upkeep (and keeping track of every reserved word in OpenQASM).

Testing done:

  • Ran tests and unit tests and did searches. I hope there's nowhere that I've missed a 'b' or 'q' used in this way because they're quite hard to search for.

Merge Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.

General

Tests

  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have checked that my tests are not configured for a specific region or account (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@krneta krneta requested review from kshitijc and ajberdy August 14, 2023 23:28
@krneta krneta requested a review from a team as a code owner August 14, 2023 23:28
@codecov
Copy link

codecov bot commented Aug 14, 2023

Codecov Report

Merging #675 (7e8d3f0) into main (885a63b) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #675   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          122       122           
  Lines         8133      8144   +11     
  Branches      1822      1826    +4     
=========================================
+ Hits          8133      8144   +11     
Files Changed Coverage Δ
src/braket/circuits/serialization.py 100.00% <ø> (ø)
src/braket/pulse/ast/qasm_transformer.py 100.00% <ø> (ø)
src/braket/circuits/circuit.py 100.00% <100.00%> (ø)
src/braket/parametric/free_parameter.py 100.00% <100.00%> (ø)

@krneta krneta merged commit b158736 into main Aug 21, 2023
@krneta krneta deleted the param_names branch August 21, 2023 16:53
krneta added a commit that referenced this pull request Sep 13, 2023
…we use for OpenQASM generation. (#675)"

This reverts commit b158736.
krneta added a commit that referenced this pull request Sep 13, 2023
…we use for OpenQASM generation. (#675)" (#701)

This reverts commit b158736.
virajvchaudhari added a commit that referenced this pull request Sep 26, 2023
* feat: add Aria2 enum (#653)

Co-authored-by: Viraj Chaudhari <[email protected]>
Co-authored-by: Cody Wang <[email protected]>

* infra: bump actions/checkout from 3.6.0 to 4.0.0 (#696)

Bumps [actions/checkout](https://github.com/actions/checkout) from 3.6.0 to 4.0.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@f43a0e5...3df4ab1)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* prepare release v1.55.0

* update development version to v1.55.1.dev0

* Revert "update: restricting parameter names to not collide with ones we use for OpenQASM generation. (#675)" (#701)

This reverts commit b158736.

* infra: update codeowner file to amazon-braket/braket-dev (#699)

Co-authored-by: Abe Coull <[email protected]>

* doc: Replace aws org with amazon-braket (#705)

* prepare release v1.55.1

* update development version to v1.55.2.dev0

* doc: change the sphinx requirement to be greater than 7.0.0 (#704)

Co-authored-by: Cody Wang <[email protected]>

* doc: add code contributors to the readme (#703)

Co-authored-by: Cody Wang <[email protected]>

* doc: Remove trailing backquotes (#706)

* infra: update the pre-commit hook with linters (#678)

* infra: update the pre-commit hook with linters and secrets check

Co-authored-by: Abe Coull <[email protected]>
Co-authored-by: Cody Wang <[email protected]>

* prepare release v1.55.1.post0

* update development version to v1.55.2.dev0

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: ashlhans <[email protected]>
Co-authored-by: Cody Wang <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: ci <ci>
Co-authored-by: Milan <[email protected]>
Co-authored-by: Angela Guo <[email protected]>
Co-authored-by: Abe Coull <[email protected]>
Co-authored-by: Abe Coull <[email protected]>
virajvchaudhari added a commit that referenced this pull request Sep 26, 2023
* feat: add queue position for tasks (#299)

* feat: add queue position for tasks

* update docstring for queue_position

* update docstrings

* update package_name

* update to enums and dataclass

* test: add integ test for task queue_position (#300)

* refactor: dataclass and file naming for queue info (#301)

* refactor: dataclass and file naming for queue info

* apply suggestions

* update: task queue position after refactor (#309)

* update: task queue position after refactor

* add queue_position type hint details, change order of info

* feat: queue position for hybrid jobs (#302)

* feat: queue position for hybrid jobs

* handle message return

* add docstring changes

* update docstring, and return None

* add context in dataclass

* update docstrings

* indent fix

* test: add integ test for jobs queue position (#310)

* feat: queue position for hybrid jobs

* handle message return

* add docstring changes

* update docstring, and return None

* add context in dataclass

* test: add integ test for jobs queue position

* remove comment

* minor fix

* remove unnecessary branching

* fix docstring merge

* remove dataclass redefinition after merge

* feat: queue depth for devices (#306)

* feat: queue depth for devices (dataclass version)

* add unit-test for queue depth

* modify docstrings, remove helper funcs

* add more info to docstrings

* minor test edit

* docstrings

* indent

* update QueuePriority to QueueType

* test: add integ test for queue_depth (#311)

* refactor: job and quantum_task keywords for queue_depth (#312)

* refactor: job and quantum_task keywords for queue_depth

* Update src/braket/aws/queue_information.py

Co-authored-by: Kshitij Chhabra <[email protected]>

---------

Co-authored-by: Kshitij Chhabra <[email protected]>

* deps: update boto3 version for queue visibility (#319)

* sync: public-main changes into feature/queue_visibility (#320)

* feat: add Aria2 enum (#653)

Co-authored-by: Viraj Chaudhari <[email protected]>
Co-authored-by: Cody Wang <[email protected]>

* infra: bump actions/checkout from 3.6.0 to 4.0.0 (#696)

Bumps [actions/checkout](https://github.com/actions/checkout) from 3.6.0 to 4.0.0.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@f43a0e5...3df4ab1)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* prepare release v1.55.0

* update development version to v1.55.1.dev0

* Revert "update: restricting parameter names to not collide with ones we use for OpenQASM generation. (#675)" (#701)

This reverts commit b158736.

* infra: update codeowner file to amazon-braket/braket-dev (#699)

Co-authored-by: Abe Coull <[email protected]>

* doc: Replace aws org with amazon-braket (#705)

* prepare release v1.55.1

* update development version to v1.55.2.dev0

* doc: change the sphinx requirement to be greater than 7.0.0 (#704)

Co-authored-by: Cody Wang <[email protected]>

* doc: add code contributors to the readme (#703)

Co-authored-by: Cody Wang <[email protected]>

* doc: Remove trailing backquotes (#706)

* infra: update the pre-commit hook with linters (#678)

* infra: update the pre-commit hook with linters and secrets check

Co-authored-by: Abe Coull <[email protected]>
Co-authored-by: Cody Wang <[email protected]>

* prepare release v1.55.1.post0

* update development version to v1.55.2.dev0

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: ashlhans <[email protected]>
Co-authored-by: Cody Wang <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: ci <ci>
Co-authored-by: Milan <[email protected]>
Co-authored-by: Angela Guo <[email protected]>
Co-authored-by: Abe Coull <[email protected]>
Co-authored-by: Abe Coull <[email protected]>

* Revert "sync: public-main changes into feature/queue_visibility (#320)"

This reverts commit be6460c.

* update github script

* minor fix

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Kshitij Chhabra <[email protected]>
Co-authored-by: ashlhans <[email protected]>
Co-authored-by: Cody Wang <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Milan <[email protected]>
Co-authored-by: Angela Guo <[email protected]>
Co-authored-by: Abe Coull <[email protected]>
Co-authored-by: Abe Coull <[email protected]>
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.

2 participants