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

fix devices.qubit.apply_operation with large tf states #3884

Merged
merged 11 commits into from
Mar 10, 2023

Conversation

albi3ro
Copy link
Contributor

@albi3ro albi3ro commented Mar 9, 2023

Tensorflow doesn't support slicing on states with more than 8 dimensions. So if we try and run the code:

state = create_initial_state(range(10), like='tensorflow')
apply_operation(qml.PauliZ(9), state)

We get:

---------------------------------------------------------------------------
UnimplementedError                        Traceback (most recent call last)
Cell In[7], line 2
      1 state = create_initial_state(range(10), like='tensorflow')
----> 2 apply_operation(qml.PauliZ(9), state)

File Python.framework/Versions/3.10/lib/python3.10/functools.py:889, in singledispatch.<locals>.wrapper(*args, **kw)
    885 if not args:
    886     raise TypeError(f'{funcname} requires at least '
    887                     '1 positional argument')
--> 889 return dispatch(args[0].__class__)(*args, **kw)

File pennylane/pennylane/devices/qubit/apply_operation.py:183, in apply_pauliz(op, state)
    180 sl_0 = _get_slice(0, op.wires[0], n_wires)
    181 sl_1 = _get_slice(1, op.wires[0], n_wires)
--> 183 state1 = math.multiply(state[sl_1], -1)
    184 return math.stack([state[sl_0], state1], axis=op.wires[0])

File /lib/python3.10/site-packages/tensorflow/python/util/traceback_utils.py:153, in filter_traceback.<locals>.error_handler(*args, **kwargs)
    151 except Exception as e:
    152   filtered_tb = _process_traceback_frames(e.__traceback__)
--> 153   raise e.with_traceback(filtered_tb) from None
    154 finally:
    155   del filtered_tb

File lib/python3.10/site-packages/tensorflow/python/framework/ops.py:7215, in raise_from_not_ok_status(e, name)
   7213 def raise_from_not_ok_status(e, name):
   7214   e.message += (" name: " + name if name is not None else "")
-> 7215   raise core._status_to_exception(e) from None

UnimplementedError: {{function_node __wrapped__StridedSlice_device_/job:localhost/replica:0/task:0/device:CPU:0}} Unhandled input dimensions 10 [Op:StridedSlice] name: strided_slice

Since this case is exceptional, and we do not want to slow down the standard logical slow, I implemented fallback logic using try-except blocks.

If the error is a tf.errors.UnimplementedError, we fall back to apply_operation_tensordot.

This will probably have performance implications for applying PauliZ and CNOT on large states, but at least it won't affect the performance in cases where the error does not occur.

Blocks testing for #3862

@albi3ro albi3ro requested a review from a team March 9, 2023 20:54
@timmysilv
Copy link
Contributor

timmysilv commented Mar 10, 2023

apply_times.txt
imo, this can just be a conditional check right at the start of the function:

n_dim = qml.math.ndim(state)
if n_dim >= 9 and qml.math.get_interface(state) == "tensorflow":
    return apply_operation_tensordot(op, state)

I think this is better for a few reasons:

  • For smaller states, this will evaluate to false immediately without dispatching to math. For larger states, this won't be the bottleneck anyway
  • The code is much clearer
  • This only checks for the exact scenario that you know causes the bug. If something we didn't foresee happens, we might silently default to tensordot without realizing or knowing why
  • I suspected that try-except is slow/not preferred in general, so I benchmarked the difference between try-except and using get_interface (with the lazy evaluation suggested above) over a range of system sizes. I attached a text file with results, but tl;dr the time is almost identical for low systems (try-except might be the teeny tiniest bit faster below 9 qubits because of the get_interface call, but it's a matter of microseconds), but notably faster for 9+ qubit systems, around 100 microseconds faster.

@albi3ro
Copy link
Contributor Author

albi3ro commented Mar 10, 2023

@timmysilv Thanks for running those benchmarks. I've updated the code to to check first.

@albi3ro albi3ro requested a review from timmysilv March 10, 2023 19:21
@codecov
Copy link

codecov bot commented Mar 10, 2023

Codecov Report

Merging #3884 (4106c59) into master (bdbd929) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3884   +/-   ##
=======================================
  Coverage   99.71%   99.71%           
=======================================
  Files         341      341           
  Lines       29810    29814    +4     
=======================================
+ Hits        29724    29728    +4     
  Misses         86       86           
Impacted Files Coverage Δ
pennylane/devices/qubit/apply_operation.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@albi3ro albi3ro requested a review from mudit2812 March 10, 2023 20:53
Copy link
Contributor

@mudit2812 mudit2812 left a comment

Choose a reason for hiding this comment

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

Looks good Christina! Are there any other interfaces such as torch that might also share the slicing trouble?

pennylane/devices/qubit/apply_operation.py Outdated Show resolved Hide resolved
pennylane/devices/qubit/apply_operation.py Outdated Show resolved Hide resolved
@albi3ro
Copy link
Contributor Author

albi3ro commented Mar 10, 2023

[sc-34873]

albi3ro and others added 2 commits March 10, 2023 16:03
Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
@albi3ro
Copy link
Contributor Author

albi3ro commented Mar 10, 2023

Looks good Christina! Are there any other interfaces such as torch that might also share the slicing trouble?

This problem was found when writing tests for #3862 . All other interfaces passed those tests.

@albi3ro albi3ro requested a review from mudit2812 March 10, 2023 21:44
Copy link
Contributor

@mudit2812 mudit2812 left a comment

Choose a reason for hiding this comment

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

Thanks Christina!

@albi3ro albi3ro enabled auto-merge (squash) March 10, 2023 23:09
@albi3ro albi3ro merged commit deb6ae3 into master Mar 10, 2023
@albi3ro albi3ro deleted the simulate-pauliz-tf branch March 10, 2023 23:28
@@ -111,7 +111,10 @@

* Made `qml.OrbitalRotation` and consequently `qml.GateFabric` consistent with the interleaved Jordan-Wigner ordering.
[(#3861)](https://github.com/PennyLaneAI/pennylane/pull/3861)


* `qml.devices.qubit.apply_operation` catches the `tf.errors.UnimplementedError` that occurs when `PauliZ` or `CNOT` gates
Copy link
Contributor

Choose a reason for hiding this comment

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

i realize this isn't the most fitting changelog entry anymore... maybe let's sneak a touch-up into another PR?

mudit2812 added a commit that referenced this pull request Apr 13, 2023
* fix apply operation with large tf states

* black and changelog

* change back order in multiply function

* check first instead of catching error

* Update pennylane/devices/qubit/apply_operation.py

Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>

* Update pennylane/devices/qubit/apply_operation.py

Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>

---------

Co-authored-by: Mudit Pandey <mudit.pandey@xanadu.ai>
@mudit2812 mudit2812 mentioned this pull request Jul 8, 2024
5 tasks
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.

3 participants