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.

Sign up for GitHub

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

Jump to bottom

Bugfix/incorrect multi line conditionals #1510

Merged
merged 13 commits into from
Aug 20, 2024
Merged

Bugfix/incorrect multi line conditionals #1510

merged 13 commits into from
Aug 20, 2024

Conversation

yao-cqc
Copy link
Contributor

@yao-cqc yao-cqc commented Jul 31, 2024 •
edited
Loading

Description

This PR fixes some issues related to converting conditional blocks to QASM.

  1. Handle converting conditional blocks.(e.g. conditional SetBits)
  2. Restrict condition simplification (i.e. replacing scratch conditions with actual predicates) to internal scratch bits (fixes QASM conversion error when multiple RangePredicate targeting the same bit #1530)
  3. Rework condition simplification.

Related issues

Fixes #1491, #1530

Checklist

  • I have performed a self-review of my code.
  • I have commented hard-to-understand parts of my code.
  • I have made corresponding changes to the public API documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have updated the changelog with any user-facing changes.

@yao-cqc yao-cqc marked this pull request as ready for review July 31, 2024 15:29
@yao-cqc yao-cqc requested a review from cqc-melf July 31, 2024 15:29
cqc-melf
cqc-melf reviewed Jul 31, 2024
View reviewed changes
pytket/tests/qasm_test.py
range_predicate = RangePredicateOp(6, 0, 27)
c = Circuit(0, 8)
c.add_gate(range_predicate, [0, 1, 2, 3, 4, 5, 6], condition=Bit(7))
with pytest.raises(Exception) as errorinfo:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe link the issue here?

pytket/tests/qasm_test.py
def test_conditional_multi_line_ops() -> None:
# https://github.com/CQCL/tket/issues/1491
c = Circuit(0, 3)
c.add_c_setbits(values=[True, True], args=[Bit(1), Bit(2)], condition=Bit(0))
Copy link
Contributor

Choose a reason for hiding this comment

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

I see the following problem with this solution, what happens if the bit of the condition is used later on?

c.add_c_setbits(values=[False, True], args=[Bit(0), Bit(2)], condition=Bit(0))

When the conversion for this example works in the same way, this could run the first line, but not the second?
Do you know what I mean?

@yao-cqc yao-cqc force-pushed the bugfix/incorrect-multi-line-conditionals branch from 44b027d to 96cab3d Compare August 19, 2024 17:06
@yao-cqc yao-cqc force-pushed the bugfix/incorrect-multi-line-conditionals branch from 96cab3d to 17ae2fe Compare August 19, 2024 19:58
@yao-cqc yao-cqc requested review from cqc-alec and cqc-melf August 20, 2024 08:38
cqc-melf
cqc-melf reviewed Aug 20, 2024
View reviewed changes
Copy link
Contributor

@cqc-melf cqc-melf left a comment

Choose a reason for hiding this comment

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

Looks good, I have a few comments / questions.

pytket/tests/qasm_test.py
c.add_gate(range_predicate, [0, 1, 2, 3, 4, 5, 6], condition=Bit(7))
with pytest.raises(Exception) as errorinfo:
circuit_to_qasm_str(c, header="hqslib1")
assert "Conditional RangePredicate is currently unsupported." in str(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this something we want to support in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we will need to support this in the future. I added a comment to remind us.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add an issue as well?

pytket/pytket/qasm/qasm.py Outdated Show resolved Hide resolved
pytket/tests/qasm_test.py Show resolved Hide resolved
@yao-cqc yao-cqc requested a review from cqc-melf August 20, 2024 13:05
cqc-melf
cqc-melf approved these changes Aug 20, 2024
View reviewed changes
Copy link
Contributor

@cqc-melf cqc-melf left a comment

Choose a reason for hiding this comment

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

Looks good!

@yao-cqc yao-cqc merged commit c2b8166 into main Aug 20, 2024
30 checks passed
@yao-cqc yao-cqc deleted the bugfix/incorrect-multi-line-conditionals branch August 20, 2024 14:31
CalMacCQ added a commit that referenced this pull request Sep 6, 2024
@CalMacCQ @cqc-melf
@DNA386 @jake-arkinstall @cqc-alec @johnchildren @yao-cqc @dependabot
docs: update docmain post 1.32 release (#1567)
a707768 
* add docs check (#1516)

* add docs check

* add poetry.lock

* circuit-display: update css file to v0.9 (#1521)

* docs: update navbar config and theming submodule (#1523)

* update navbar config

* use latest quantinuum-sphinx

* fix some local build warnings

* bump quantinuum-sphinx (#1525)

* release 1.31.1 (#1522)

* update version and changelog

* Update pytket/docs/changelog.rst

* correct navbar links (#1527)

* Updated flake.lock to bring symengine-0.12.0 into nix (#1423)

* Update to pybind11 2.13.3 (#1531)

* Fix symbol substitution for classical operations (#1538)

* Update pybind11 and catch2 versions (#1539)

* docs: Add pytket-azure to list of extensions (#1540)

* feat: Refactor WASM module (#1503)

* feat: Refactor WASM module

- Add a new WasmModuleHandler class that takes raw wasm_module bytes on construction, but has the same interface as WasmFileHandler.
- WasmFileHandler now inherits from WasmModuleHandler.
- Split checking of the WASM module into a new function so it can be called after initialisation if required.
- Adds new cached properties, bytecode, bytecode_base64 and module_uid which allow lazy loading of the module in encoded form, or computation of a unique identifier for the module.
- Add deprecated properties _check_file, wasm_file_encoded and _wasmfileuid to maintain compatibility with existing versions of pytket-qir and pytket-quantinuum.
- Remove some exceptions that are no longer possible.

* Bugfix/incorrect multi line conditionals (#1510)

* Add failing tests

* Delay declaring registers

* Handle conditional blocks

* Raise error for conditional `RangePredicate`

* Refactor `mark_as_written`

* Refactor `self.range_preds`

* Replacing conditions with predicates

* Add changelog entry

* Format code

* Fix typing issue

* Add testcase for multi-bit condition

* Remove unnecessary 0 assignment

* reposition comment in test

* Fix order of `lower` and `upper` in constructing `RangePredicateOp` (#1549)

* fix: Add copy implementations for Unit IDs (#1550)

* Bump slackapi/slack-github-action from 1.26.0 to 1.27.0 (#1555)

Bumps [slackapi/slack-github-action](https://github.com/slackapi/slack-github-action) from 1.26.0 to 1.27.0.
- [Release notes](https://github.com/slackapi/slack-github-action/releases)
- [Commits](slackapi/slack-github-action@v1.26.0...v1.27.0)

---
updated-dependencies:
- dependency-name: slackapi/slack-github-action
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

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

* Update version and changelog. (#1557)

* Update to pybind11 2.13.5 (#1559)

* Update libs to boost 1.86.0. (#1560)

* Install pytket requirements before version consistency checks. (#1562)

* Run valgrind on ubuntu-24.04 (#1563)

* Allow constant ZZPhase fidelity in DecomposeTK2 (#1558)

feat: Allow constant ZZPhase fidelity in DecomposeTK2

Currently it is not possible to serialize the Decompose TK2 pass as it can include an arbitrary function. This commit
allows a constant float value in addition, which will allow serialization of the default compilation pass for pytket-quantinuum.

---------

Co-authored-by: Alec Edgington <[email protected]>

* Use correct conan profile for valgrind build. (#1564)

* Update to boost 1.86.0, tktokenswap 0.3.9 and tkwsm 0.3.9 (#1561)

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: cqc-melf <[email protected]>
Co-authored-by: Tiffany Duneau <[email protected]>
Co-authored-by: Jake Arkinstall <[email protected]>
Co-authored-by: Alec Edgington <[email protected]>
Co-authored-by: John Children <[email protected]>
Co-authored-by: yao-cqc <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Alec Edgington <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cqc-melf cqc-melf cqc-melf approved these changes

@cqc-alec cqc-alec cqc-alec approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@yao-cqc @cqc-alec @cqc-melf