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

[Refact] Get rid of PyQTimeDependentEvolution #568

Merged
merged 10 commits into from
Oct 11, 2024
Merged

Conversation

vytautas-a
Copy link
Collaborator

Substitute PyQTimeDependentEvolution class with proper call to pyqtorch's time-dependent functionality.

@vytautas-a
Copy link
Collaborator Author

This refactoring disables the embedding function call for pyqtorch backend, thus currently only HamEvo block works properly. I suggest we could completely get rid of the embedding call when dealing with pyqtorch. This can be achieved by utilizing the new sympy -> ConcretizedCallable converter.

@RolandMacDoland
Copy link
Collaborator

Hey @vytautas-a can I mark this PR as ready ?

@RolandMacDoland RolandMacDoland added the refactoring Refactoring of legacy code label Sep 12, 2024
@vytautas-a
Copy link
Collaborator Author

@Roland-djee, well this PR here is currently more in the proposal stage. It depends also on another PR that I'll open in pyqtorch.

Copy link
Collaborator

@RolandMacDoland RolandMacDoland left a comment

Choose a reason for hiding this comment

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

Thanks a lot @vytautas-a looks nice and clean. Minor renaming suggestions and comments. Will approve once addressed.

qadence/backends/pyqtorch/convert_ops.py Outdated Show resolved Hide resolved
qadence/backends/pyqtorch/convert_ops.py Outdated Show resolved Hide resolved
qadence/backends/pyqtorch/convert_ops.py Outdated Show resolved Hide resolved
qadence/backends/pyqtorch/convert_ops.py Outdated Show resolved Hide resolved
qadence/backends/pyqtorch/convert_ops.py Show resolved Hide resolved
qadence/backends/pyqtorch/convert_ops.py Show resolved Hide resolved
qadence/backends/pyqtorch/convert_ops.py Outdated Show resolved Hide resolved
qadence/execution.py Outdated Show resolved Hide resolved
@vytautas-a vytautas-a marked this pull request as ready for review October 11, 2024 09:32
vytautas-a and others added 3 commits October 11, 2024 12:34
Co-authored-by: RolandMacDoland <9250798+RolandMacDoland@users.noreply.github.com>
Co-authored-by: RolandMacDoland <9250798+RolandMacDoland@users.noreply.github.com>
Copy link
Collaborator

@jpmoutinho jpmoutinho left a comment

Choose a reason for hiding this comment

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

Thanks @vytautas-a :)

Few minor things and then it should be good to go.

One thing, I see that the docs for the time-dependent HamEvo reads:

# create parameterized HamEvo block
hamevo = HamEvo(generator_td, 0.0, duration=duration)

Is passing 0.0 correct there? I thought the idea was to pass the time parameter in that argument?

qadence/backends/pyqtorch/convert_ops.py Outdated Show resolved Hide resolved
tests/engines/test_torch.py Outdated Show resolved Hide resolved
tests/qadence/test_quantum_model.py Outdated Show resolved Hide resolved
tests/qadence/test_quantum_model.py Outdated Show resolved Hide resolved
@vytautas-a
Copy link
Collaborator Author

Thanks @vytautas-a :)

Few minor things and then it should be good to go.

One thing, I see that the docs for the time-dependent HamEvo reads:

# create parameterized HamEvo block
hamevo = HamEvo(generator_td, 0.0, duration=duration)

Is passing 0.0 correct there? I thought the idea was to pass the time parameter in that argument?

Yes, the docs have to updated.

Copy link
Collaborator

@jpmoutinho jpmoutinho left a comment

Choose a reason for hiding this comment

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

LGTM!

@vytautas-a vytautas-a merged commit 71a573b into main Oct 11, 2024
8 checks passed
@vytautas-a vytautas-a deleted the va/time-dep-hamevo branch October 11, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Refactoring of legacy code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants