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

Add parameter batch support to the AmplitudeEmbedding template #1761

Merged
merged 14 commits into from
Oct 19, 2021
Merged

Add parameter batch support to the AmplitudeEmbedding template #1761

merged 14 commits into from
Oct 19, 2021

Conversation

dime10
Copy link
Contributor

@dime10 dime10 commented Oct 17, 2021

Context: In PR #1710, support for creating PennyLane QNodes with batches of input parameters via @qml.batch_params was added. However, not all templates have support for this feature yet.

Description of the Change:

  • This modifies the AmplitudeEmbedding template to support a batch of parameters
    for the 'features' input. The pre-processing on the inputs is then applied for each
    feature_set in the batch.
  • It adds the AmplitudeEmbedding template to the simple circuit test of the
    test_batch_params.py file. Also removes a test in test_amplitude.py that is no longer
    applicable.
  • Fixes the wording in one the error messages of the template (unrelated to issue).

Benefits: It's possible to use batched parameters on the AmplitudeEmbedding template.

Possible Drawbacks: Less robust input checks.

Related GitHub Issues: Closes #1745

This modifies the AmplitudeEmbedding template to support a batch
of parameters for the 'features' input. The pre-processing on the
inputs is then applied for each feature_set in the batch.
@dime10
Copy link
Contributor Author

dime10 commented Oct 17, 2021

@josh146 An existing test, test_if_features_wrong_shape, in test_amplitude.py fails since it specifically checks that an error is thrown if features has more than one dimension. You mentioned adding tests to test_batch_params.py, but we'd also need to either remove this test, or have it check for >2 dimensions.

The other thing I was considering is that since the template is not aware of whether the @qml.batch_params decorator is actually present, we would not catch a situation where features is 2 dimensional without batching, which would previously throw an error. For example, this circuit should throw an error but now won't:

@qml.beta.qnode(dev)
def circuit(data, weights):
    qml.templates.AmplitudeEmbedding([[1,1,1,1], [2,2,2,2]], wires=[0, 1])

dime10 and others added 5 commits October 17, 2021 15:34
Changes `test_throws_exception_if_features_wrong_shape` to only throw
an error if the # of dimensions of the features tensor is >2, instead
of the previous >1. This allows batched parameters to be passed.
@josh146 josh146 self-requested a review October 18, 2021 06:01
@josh146 josh146 added the WIP 🚧 Work-in-progress label Oct 18, 2021
@josh146
Copy link
Member

josh146 commented Oct 18, 2021

Thanks @dime10!

test_if_features_wrong_shape, in test_amplitude.py fails since it specifically checks that an error is thrown if features has more than one dimension. We'd also need to either remove this test, or have it check for >2 dimensions.

Yes, this is a very good catch. I ran into the same issue recently updating the StronglyEntanglingLayer to support batches of parameters, and I'm not sure I can think of a good solution. As you say, since the operation does not know ahead of time whether it is in a batched mode or not, it doesn't seem possible to validate this any more.

For now, I suggest removing this validation (and the corresponding test_if_features_wrong_shape).

For example, this circuit should throw an error but now won't:

Yes, this one deserves more thought. No need to worry about it now for this PR, but something we should consider and fix in a subsequent PR 🙂

We are currently working on rethinking the Operator class (tagging @mariaschuld for visibility), and potentially one solution would be to make the Operator class aware of potential batch dimensions ahead of time, e.g.,

@qml.beta.qnode(dev)
def circuit(data, weights):
    qml.templates.AmplitudeEmbedding([[1,1,1,1], [2,2,2,2]], wires=[0, 1], batch_axes=0)

In the meantime @dime10, let me know if you have any more questions, or otherwise, if the code is ready to be reviewed!

@codecov
Copy link

codecov bot commented Oct 18, 2021

Codecov Report

Merging #1761 (7350486) into master (1bca151) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1761   +/-   ##
=======================================
  Coverage   98.90%   98.90%           
=======================================
  Files         206      206           
  Lines       15385    15388    +3     
=======================================
+ Hits        15216    15219    +3     
  Misses        169      169           
Impacted Files Coverage Δ
pennylane/templates/embeddings/amplitude.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1bca151...7350486. Read the comment docs.

The `test_throws_exception_if_features_wrong_shape` is no  longer
applicable with batching, since it verifies that the template does
not accept feature tensors with more than one dimension.
@dime10 dime10 changed the title [WIP] Add parameter batch support to additional templates Add parameter batch support to the AmplitudeEmbedding template Oct 18, 2021
@dime10
Copy link
Contributor Author

dime10 commented Oct 18, 2021

@josh146 This is ready for review. Thank you for answering my questions!

@josh146
Copy link
Member

josh146 commented Oct 18, 2021

Thanks @dime10! I will have a review out before the end of the day 🙂

@josh146 josh146 added review-ready 👌 PRs which are ready for review by someone from the core team. and removed WIP 🚧 Work-in-progress labels Oct 19, 2021
Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

Great work @dime10, very nice as well to see that you have kept in the length normalization! I liked your solution re: iterating over the batch dimension - I suppose this works since we are under the assumption that there is only one batch dimension present.

I think this is good to go on my end, just two final changes needed:

  • From my reading, I think the test test_throws_exception_if_features_wrong_shape can be 'undeleted'.
  • Don't forget to add your name, and a brief description of the change here, to the changelog! The changelog is in doc/changelog-dev.md.

pennylane/templates/embeddings/amplitude.py Outdated Show resolved Hide resolved
pennylane/templates/embeddings/amplitude.py Show resolved Hide resolved
tests/transforms/test_batch_params.py Show resolved Hide resolved
dime10 and others added 2 commits October 19, 2021 13:41
Co-authored-by: Josh Izaac <josh146@gmail.com>
Copy link
Member

@josh146 josh146 left a comment

Choose a reason for hiding this comment

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

@dime10 this looks great, happy to approve and merge it in!

Just before I do, don't forget to add a link to this PR in the changelog (it can be added to this existing entry

[(#1710)](https://github.com/PennyLaneAI/pennylane/pull/1710)
), and to add your name to the contributors section 🙂

@josh146 josh146 merged commit 5aa93dd into PennyLaneAI:master Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-ready 👌 PRs which are ready for review by someone from the core team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add batching support to the AmplitudeEmbedding template
2 participants