pennylane icon indicating copy to clipboard operation
pennylane copied to clipboard

Raise an error or warning in qml.compile if basis_set contains operations instead of strings

Open AnuravModak opened this issue 1 year ago • 6 comments

Towards #6132

This PR includes the following changes:

If the basis set contains any items that are not strings or subclasses of qml.operation.Operator, a ValueError will be raised. Additionally, if the basis set contains operator types instead of names and results in an empty set, an error will be raised.

AnuravModak avatar Aug 24 '24 17:08 AnuravModak

Codecov Report

Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Project coverage is 99.66%. Comparing base (1f55c88) to head (9d212f8). Report is 27 commits behind head on master.

Files Patch % Lines
pennylane/transforms/compile.py 83.33% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6137      +/-   ##
==========================================
- Coverage   99.67%   99.66%   -0.02%     
==========================================
  Files         432      445      +13     
  Lines       41839    42363     +524     
==========================================
+ Hits        41702    42220     +518     
- Misses        137      143       +6     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Aug 24 '24 17:08 codecov[bot]

Hey @albi3ro , i have added the required code changes and mandatory tests, to throw an error for the prescribed scenario, kindly review it and let me know if any thing else has to be done on coding or implementation part!

AnuravModak avatar Aug 26 '24 17:08 AnuravModak

Thanks for opening this PR @AnuravModak .

Sorry if there was confusion on the issue, but we don't currently support to contain operator types like:

qml.compile(circuit, basis_set={qml.X, qml.Y, qml.Z, qml.RX, qml.RY, qml.RZ})

Part of the suggestion was to start allowing such a syntax. Do you think you could expand the scope of the PR to include that change? If not, we should also raise an error if basis_set includes operator types.

albi3ro avatar Aug 26 '24 18:08 albi3ro

Hey @albi3ro , thanks for clarifying! Now i have made the necessary changes allowing such syntax:

Now the compile transform will handle both operation names and types within the basis_set. Also the tests i have written checks this by using a simple quantum function with an RX operation. By setting the basis_set to ["RX"], the test ensures that the compile transform correctly stops at the RX operation and doesn't alter it, validating that our changes work as expected.

let me know if it matches your expectations!

AnuravModak avatar Aug 27 '24 16:08 AnuravModak

Looks like it still doesn't accept class types as a valid way of specifying the basis set. Maybe we should limit the scope to just validating that the basis set only contains strings, and then do a follow up with the new feature allowing the specification of the basis set via class types.

The feature we are looking for would be allowing:

tape = qml.tape.QuantumScript([qml.Rot(1.2, 2.3, 3.4, 0)], [qml.probs()])
qml.compile(tape, basis_set=[qml.RX, qml.RY, qml.RZ])

Which we would expect to decompose the Rot into RY and RZ operations.

albi3ro avatar Aug 27 '24 17:08 albi3ro

Looks like it still doesn't accept class types as a valid way of specifying the basis set. Maybe we should limit the scope to just validating that the basis set only contains strings, and then do a follow up with the new feature allowing the specification of the basis set via class types.

The feature we are looking for would be allowing:

tape = qml.tape.QuantumScript([qml.Rot(1.2, 2.3, 3.4, 0)], [qml.probs()])
qml.compile(tape, basis_set=[qml.RX, qml.RY, qml.RZ])

Which we would expect to decompose the Rot into RY and RZ operations.

Hi @albi3ro ,

I think my new changes may align closer to your requrements:

Handling Class Types in basis_set:

  1. The code now checks if elements in the basis_set are either string-based names or operator class types.
  2. It converts the class types into their corresponding operation names and merges them with any string-based names.

Stopping Condition: The stopping condition in stop_at now correctly checks whether an operation matches the names or types specified in basis_set

Now also i have deployed the code in my local and have tested it!

Let me know if the results are right or not!


def test_empty_basis_set():
    # Define a quantum tape with a Rot gate
    tape = qml.tape.QuantumScript([qml.Rot(1.2, 2.3, 3.4, 0)], [qml.probs()])

    # Create a basis set with only invalid types (e.g., an integer and a float)
    invalid_basis_set = [123, 4.56]

    try:
        # Attempt to compile with an invalid basis_set, expecting a ValueError
        qml.compile(tape, basis_set=invalid_basis_set)
    except ValueError as e:
        # Print the error message to confirm it was raised
        print("Caught expected ValueError:", e)

# Run the test
test_empty_basis_set()

output:

Caught expected ValueError: basis_set contains no valid operation names or types.

Or should i just simply implement this change that you have suggested?

class_types = tuple(o for o in basis_set if isinstance(o, type)
class_names = set(o for o in basis_set if isinstance(o, str))
def stopping_condition(obj):
    return obj.name in class_names or isinstance(obj, class_types)

AnuravModak avatar Sep 01 '24 18:09 AnuravModak

Hey @AnuravModak, are you still interested in driving this PR to conclusion? Please let us know if you are still interested. Otherwise, we'll be happy to take over.

Alex-Preciado avatar Feb 25 '25 18:02 Alex-Preciado

Hi @AnuravModak , just wanted to follow up on my message from last week regarding this PR. Please let us know if you're still interested in moving forward.

Alex-Preciado avatar Mar 03 '25 17:03 Alex-Preciado

Hi @AnuravModak , since we haven’t heard back, we’ll go ahead and close this PR for now. If you’d like to revisit it in the future, feel free to reopen or submit a new PR. Thanks for your contributions to PennyLane! 😊

Alex-Preciado avatar Mar 11 '25 02:03 Alex-Preciado