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

CircuitSimulator Refactor #12

Merged
merged 5 commits into from
Apr 11, 2023
Merged

Conversation

amccaskey
Copy link
Collaborator

@amccaskey amccaskey commented Mar 22, 2023

Add applyGate method for subclasses to implement instead of requiring overloads for all gate methods.

Introduce gate queue to better support mid-circuit measurement and data persistence to named classical registers.

Introduce unique cached circuit name to support sub-type caching strategies (e.g. cache the optimal contraction path for a given circuit).

Break up the python quake file check tests into separate ones for easier debugging.

@github-actions
Copy link

github-actions bot commented Mar 22, 2023

CLA Assistant Lite bot All Contributors have signed the CLA.

@amccaskey
Copy link
Collaborator Author

I have read the Contributor License Agreement and I hereby accept the Terms.

@amccaskey amccaskey marked this pull request as draft March 22, 2023 14:03
@amccaskey amccaskey marked this pull request as ready for review March 22, 2023 20:34
@amccaskey amccaskey changed the title [WIP] CircuitSimulator Refactor CircuitSimulator Refactor Mar 22, 2023
@bettinaheim
Copy link
Collaborator

@boschmitt Could you please review this?
@DmitryLyakh @ahehn-nv as optional.

Copy link
Collaborator

@boschmitt boschmitt left a comment

Choose a reason for hiding this comment

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

I like the refactoring, it does simplifies the code. However, it seems that the set of changes is orthogonal; They could have been broken into smaller, more focused PRs (might still be worth doing it).

Breaking up the big python test into smaller ones is a great move, but I would still keep similar ones on the same file, specifically leaving all qalloc tests in the same file.

The fix in the clang-format configuration file is likely to affect the whole code base, it certainly worth creating a separate PR that with the fix and rerun in the entire codebase.

lib/Target/IQM/TranslateToIQMJson.cpp Outdated Show resolved Hide resolved
runtime/nvqir/CircuitSimulator.h Outdated Show resolved Hide resolved
runtime/nvqir/CircuitSimulator.h Outdated Show resolved Hide resolved
runtime/nvqir/CircuitSimulator.h Outdated Show resolved Hide resolved
runtime/nvqir/CircuitSimulator.h Outdated Show resolved Hide resolved
runtime/nvqir/qpp/QppDMCircuitSimulator.cpp Outdated Show resolved Hide resolved
python/tests/compiler/no_input.py Outdated Show resolved Hide resolved
@bettinaheim bettinaheim added the release notes Changes need to be captured in the release notes label Apr 11, 2023
…mulation types

Signed-off-by: Alex McCaskey <amccaskey@nvidia.com>
Signed-off-by: Alex McCaskey <amccaskey@nvidia.com>
Signed-off-by: Alex McCaskey <amccaskey@nvidia.com>
Signed-off-by: Alex McCaskey <amccaskey@nvidia.com>
Copy link
Collaborator

@boschmitt boschmitt left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for this work Alex!

runtime/nvqir/CircuitSimulator.h Outdated Show resolved Hide resolved
Signed-off-by: Alex McCaskey <amccaskey@nvidia.com>
@amccaskey amccaskey merged commit ee2e5d1 into NVIDIA:main Apr 11, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
@amccaskey amccaskey deleted the circuitSimulatorRefactor branch May 26, 2023 16:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release notes Changes need to be captured in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants