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 error if differentiating diff_method=None #6770

Merged
merged 18 commits into from
Jan 15, 2025

Conversation

albi3ro
Copy link
Contributor

@albi3ro albi3ro commented Jan 7, 2025

Context:

If someone tries to differentiate a qnode that has diff_method=None, they may get less-than-useful error message. We also assume that no interface parameters are used with diff_method=None

Description of the Change:

Uses the ML boundary when diff_method=None and raises an informative error if differentiation is requested.

Benefits:

ML parameters can be used with diff_method=None, and incorrect usage provides good error messages.

Possible Drawbacks:

Related GitHub Issues:

Solves #6769 [sc-81490]

Copy link
Contributor

github-actions bot commented Jan 7, 2025

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@albi3ro albi3ro changed the base branch from master to no-interface-boundary January 8, 2025 20:43
@PietropaoloFrisoni
Copy link
Contributor

@albi3ro Should this be part of the release?

@albi3ro
Copy link
Contributor Author

albi3ro commented Jan 8, 2025

@albi3ro Should this be part of the release?

Nope. It's changing architectural components and core behavior.

Base automatically changed from no-interface-boundary to master January 15, 2025 19:59
albi3ro added a commit that referenced this pull request Jan 15, 2025
…ck` (#6788)

**Context:**

While we have logic for sampling with jax, it does not really integrate
very well into the workflow. While you can technically set
`diff_method=None` right now and jit the execution end-to-end, trying to
jit `diff_method=None` will cause incomprehensible error messages on
non-DQ devices.

We want to *forbid* differentiation `diff_method=None`, but keep a way
to jit a finite shot execution.

**Description of the Change:**

In order to allow jitting finite shot executions, we need a way for the
device to be able to configure whether or not the data is converted to
numpy. To do so, we simply add another property to the
`ExecutionConfig`, `convert_to_numpy`. If `False`, then we will not use
a `pure_callback` to convert the parameters to numpy. If `True`, we use
a `pure_callback` and convert the parameters to numpy.

**Benefits:**

Speed ups due to being able to jit the entire execution.


![image](https://github.com/user-attachments/assets/738076c6-7bb5-4c38-a8cc-97e138325dbc)


**Possible Drawbacks:**

`ExecutionConfig` gets an addtional property, making it more
complicated.

**Related GitHub Issues:**

Fixes #6054 Fixes #3259  Blocks #6770

---------

Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
@albi3ro albi3ro changed the title Add error if differentiation diff_method=None Add error if differentiating diff_method=None Jan 15, 2025
Copy link

codecov bot commented Jan 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.60%. Comparing base (0bed5e8) to head (100b25c).
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6770   +/-   ##
=======================================
  Coverage   99.60%   99.60%           
=======================================
  Files         476      476           
  Lines       45178    45181    +3     
=======================================
+ Hits        44999    45002    +3     
  Misses        179      179           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@andrijapau andrijapau left a comment

Choose a reason for hiding this comment

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

This is great 🚀 !

Copy link
Contributor

@JerryChen97 JerryChen97 left a comment

Choose a reason for hiding this comment

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

LGTM! Just some good-to-have suggestions

doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
pennylane/workflow/jacobian_products.py Outdated Show resolved Hide resolved
tests/gradients/core/test_pulse_gradient.py Outdated Show resolved Hide resolved
albi3ro and others added 2 commits January 15, 2025 17:38
@albi3ro albi3ro enabled auto-merge (squash) January 15, 2025 22:46
@albi3ro albi3ro merged commit 893198a into master Jan 15, 2025
46 checks passed
@albi3ro albi3ro deleted the no-grad-on-diff-method-none branch January 15, 2025 23:21
albi3ro added a commit that referenced this pull request Jan 16, 2025
**Context:**

PR #6770 accidentally broke lightning, becasue we are now sending numpy
data to lightning instead of autograd. The autograd arrays had slightly
different casting rules. With numpy, we are no longer implicitly casting
to integers.

**Description of the Change:**

Always cast the input to `BasisState` to integers.

**Benefits:**

**Possible Drawbacks:**

**Related GitHub Issues:**

---------

Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
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.

4 participants