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

Feature/classical conditionals #152

Merged
merged 41 commits into from
Sep 23, 2024
Merged

Conversation

PabloAndresCQ
Copy link
Collaborator

@PabloAndresCQ PabloAndresCQ commented Aug 23, 2024

Description

Adding support so that simulate can:

Also included some bugfixes of MPSxMPO (I had missed calling _flush() in the newer functions).

Related issues

#151
#156

Checklist

  • I have run the tests on a device with GPUs.
  • 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.

…emoved the _sort_gates preprocessing, since it would need to be updated.
… Added recursive evaluation of logical expressions.
…d a SegFault which I believe comes from creating a Command manually
@@ -73,48 +74,68 @@ def simulate(
circuit: The pytket circuit to be simulated.
algorithm: Choose between the values of the ``SimulationAlgorithm`` enum.
config: The configuration object for simulation.
compilation_params: Experimental feature. Defaults to no compilation.
Parameters currently not documented.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is now hiding two options that I have decided to deprecate:

  • sort_gates that was always applied when calling simulate. When set to True, this is done by _get_sorted_gates (see its docstring). This code is not immediately compatible with classical operations and, although it could be adapted, this is not a priority. I am not convinced it was making a noticeable impact in simulation time for MPS, and for TTN this was probably a bit too naive. The code is not particularly easy to maintain and extend, so I am leaning towards deprecating it by "hiding it" and setting it to False by default.
  • use_kahypar used to be a option in Config, but it makes more sense to set it here, since it is the only place where it is used. Moreover, it became clear that further work would be required to include KaHyPar in the default pipeline (due to dependencies), so it was already set to False by default.

# GEQ and NOT, since these do not return int, so I am unsure what the
# semantic is meant to be.
# TODO: Similarly, it is not clear what to do with overflow of ADD, etc.
# so I have decided to not support them for now.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The semantics are not agreed upon across devices and emulators. For the time being, I'd rather error out as currently done. The current implementation should be capable of running any circuit that comes from a QASM file (parsed via pytket.qasm) using the standard QASM library. These extra operators on registers come from our internal extension to QASM, hqslib1.inc.

@PabloAndresCQ PabloAndresCQ marked this pull request as ready for review September 12, 2024 16:06
tests/test_structured_state_conditionals.py Outdated Show resolved Hide resolved
input_bits = args[: len(output_bits)]
for i, o in zip(input_bits, output_bits):
assert isinstance(i, Bit)
bits_dict[o] = bits_dict[i]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There was a silly bug here. Now fixed.

Copy link
Collaborator

@jake-arkinstall jake-arkinstall left a comment

Choose a reason for hiding this comment

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

Changes are clearly useful, and tests pass in nix (aside from one in which fidelity is better than expected, which we will cover in a separate PR by adding lower bounds to tests rather than approx checks).

@PabloAndresCQ PabloAndresCQ merged commit b1dae66 into main Sep 23, 2024
8 checks passed
@PabloAndresCQ PabloAndresCQ deleted the feature/classical_conditionals branch September 23, 2024 15:15
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