pennylane icon indicating copy to clipboard operation
pennylane copied to clipboard

Update measurement process constructor function signatures

Open mudit2812 opened this issue 1 year ago • 4 comments
trafficstars

Context: Measurement process constructor functions (qml.sample, qml.probs, etc) are inconsistent with respect to the order of the arguments given to them, which causes problems like potentially using observables as wires. This PR aims to update the functions to dispatch based on argument type over argument name.

Description of the Change:

  • Generalize function signatures to *args, **kwargs.
  • Use the type of the provided argument to determine how to use it.
  • If the name of the argument does not match the type (eg. wires=qml.PauliZ(0)), raise a warning.
  • Add an mv argument to the constructor functions.
  • Update tests to use mv=[MeasurementValue or list of MeasurementValues] instead of op=[MeasurementValue or list of MeasurementValues] (This is a big chunk of the diff, so reviewers please keep in mind that most of the files changed are tests with 2-3 lines changed).

Benefits: There are no longer inconsistencies with respect to argument name vs type.

Possible Drawbacks: mv argument is a breaking change.

Relevant Shortcut Stories: [sc-43672]

mudit2812 avatar Feb 16 '24 09:02 mudit2812

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.65%. Comparing base (d7593a3) to head (4f176c9).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5224      +/-   ##
==========================================
- Coverage   99.65%   99.65%   -0.01%     
==========================================
  Files         399      399              
  Lines       36890    36712     -178     
==========================================
- Hits        36764    36584     -180     
- Misses        126      128       +2     

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

codecov[bot] avatar Feb 16 '24 15:02 codecov[bot]

[sc-43672]

mudit2812 avatar Feb 16 '24 16:02 mudit2812

This story is currently in our backlog. @mudit2812 when we do revisit the story, do you imagine we'd use this current PR or start afresh? If the latter, we could close this PR.

trbromley avatar Jul 22 '24 12:07 trbromley

@trbromley I think this PR is still viable. I'll definitely have a better idea once I update the branch with the latest master.

mudit2812 avatar Jul 22 '24 12:07 mudit2812