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] Add Operator Product #156

Merged
merged 27 commits into from
Apr 24, 2024
Merged

[Feature] Add Operator Product #156

merged 27 commits into from
Apr 24, 2024

Conversation

EthanObadia
Copy link
Collaborator

@EthanObadia EthanObadia commented Apr 5, 2024

Add promote_operator() function to pyqtorch/utils.py
Add operator_product() function to pyqtorch/apply.py

As @jpmoutinho said here, this version of promote_operator() can be optimize. So, I believe that its optimization will be the object of an other PR later.

Closes #153
Closes #154

@EthanObadia EthanObadia self-assigned this Apr 5, 2024
@EthanObadia EthanObadia added the feature New feature or request label Apr 5, 2024
@EthanObadia EthanObadia changed the title add apply_ope_ope() function (#153) [Feature] Operators Multiplication Apr 5, 2024
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.

Looking good. Few comments from me.

pyqtorch/apply.py Outdated Show resolved Hide resolved
pyqtorch/apply.py Outdated Show resolved Hide resolved
pyqtorch/utils.py Outdated Show resolved Hide resolved
pyqtorch/utils.py Outdated Show resolved Hide resolved
pyqtorch/utils.py Outdated Show resolved Hide resolved
@dominikandreasseitz dominikandreasseitz changed the title [Feature] Operators Multiplication [Feature] Add Operator Product Apr 16, 2024
@EthanObadia EthanObadia changed the title [Feature] Add Operator Product [Feature] Add Operator Product [1] Apr 16, 2024
@EthanObadia EthanObadia changed the title [Feature] Add Operator Product [1] [Feature] Add Operator Product [0] Apr 16, 2024
@EthanObadia EthanObadia marked this pull request as ready for review April 16, 2024 13:27
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.

Nice work @EthanObadia looking good. Will approve after you've addressed these very last comments.

pyqtorch/apply.py Outdated Show resolved Hide resolved
pyqtorch/apply.py Outdated Show resolved Hide resolved
pyqtorch/apply.py Outdated Show resolved Hide resolved
pyqtorch/primitive.py Outdated Show resolved Hide resolved
pyqtorch/primitive.py Outdated Show resolved Hide resolved
pyqtorch/primitive.py Outdated Show resolved Hide resolved
pyqtorch/utils.py Outdated Show resolved Hide resolved
tests/test_digital.py Outdated Show resolved Hide resolved
pyqtorch/apply.py Outdated Show resolved Hide resolved
pyqtorch/utils.py Outdated Show resolved Hide resolved
pyqtorch/utils.py Show resolved Hide resolved
pyqtorch/apply.py Outdated Show resolved Hide resolved
pyqtorch/apply.py Outdated Show resolved Hide resolved
pyqtorch/apply.py Outdated Show resolved Hide resolved
tests/test_digital.py Outdated Show resolved Hide resolved
@EthanObadia EthanObadia changed the title [Feature] Add Operator Product [0] [Feature] Add Operator Product Apr 17, 2024
Copy link
Collaborator

@dominikandreasseitz dominikandreasseitz left a comment

Choose a reason for hiding this comment

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

lgtm

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.

Hey @EthanObadia good job ! Looking good to me. Minor typing comments.

tests/test_digital.py Outdated Show resolved Hide resolved
tests/test_digital.py Outdated Show resolved Hide resolved
pyqtorch/apply.py Outdated Show resolved Hide resolved
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. I opened an issue to keep track of the possible optimization of the operator multiplication. We can discuss it at some point.

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.

Looking good. Nice work @EthanObadia. Very final comments from my over-pedantic mindset. Will approve anyway.

pyqtorch/primitive.py Outdated Show resolved Hide resolved
tests/test_digital.py Outdated Show resolved Hide resolved
tests/test_digital.py Outdated Show resolved Hide resolved
@EthanObadia EthanObadia merged commit 0df2af5 into main Apr 24, 2024
9 checks passed
@EthanObadia EthanObadia deleted the eo/apply_ope_ope branch April 24, 2024 13:49
@EthanObadia EthanObadia added the noise noisy simulation label May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request noise noisy simulation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Allow different size operators to multiply each other [Feature] Add operator_product
5 participants