pennylane
                                
                                
                                
                                    pennylane copied to clipboard
                            
                            
                            
                        Measured wires on qscript
Before submitting
Please complete the following checklist when submitting a PR:
- 
[✓] All new features must include a unit test. If you've fixed a bug or added code that should be tested, add a test to the test directory!
 - 
[✓] All new functions and code must be clearly commented and documented. If you do make documentation changes, make sure that the docs build and render correctly by running
make docs. - 
[✓] Ensure that the test suite passes, by running
make test. - 
[✓] Add a new entry to the
doc/releases/changelog-dev.mdfile, summarizing the change, and including a link back to the PR. - 
[✓] The PennyLane source code conforms to PEP8 standards. We check all of our code against Pylint. To lint modified files, simply
pip install pylint, and then runpylint pennylane/path/to/file.py. 
When all the above are checked, delete everything above the dashed line and fill in the pull request template.
Context: A nice-to-have feature of the QuantumScript requested in issue #3203.
Description of the Change:  Adds a cached property to QuantumScript for the measured wires.
Benefits: The wires can be referenced instead of re-calculated, for example in _update_observables and QuantumScript().measured_wires.
Possible Drawbacks: The data footprint of a QuantumScript object is now larger.
Related GitHub Issues: https://github.com/PennyLaneAI/pennylane/issues/3203
Thanks @comp-phys-marc tada
Make sure you also add your name to the contributors section at the bottom of the changelog and add a link to the entry. This kind of change should also fall under "Improvements" rather than "New features". New Features tend to be larger, user-focused things.
While this is how we currently tend to treat a lot of the properties in
QuantumScript, this may be a good place where we can start using a more "lazy" approach to the properties.We currently create a lot of "intermediate" quantum scripts, that are then transformed into something else. So for the vast majority of quantum scripts created, we won't be ever using
measured_wires, so computing it is wasted time.So I'd like to consider only computing
measured_wireswhen we actually are going to use it. It's fairly lightweight, so this probably isn't that big of a deal, but might as well anyway.Some options for being more lazy:
1. Recalculate it every time we need it: See [`samples_computational_basis`](https://github.com/PennyLaneAI/pennylane/blob/597beac35f49874a042f6e0dc61bdff69cce19a3/pennylane/tape/qscript.py#L313) 2. Leave `_measured_wires` as `None` till we calculate it for the first time: See [`specs`](https://github.com/PennyLaneAI/pennylane/blob/597beac35f49874a042f6e0dc61bdff69cce19a3/pennylane/tape/qscript.py#L1131) or `graph`. 3. Use `functools.cached_property`. I haven't tried this out yet myself. But its an option.I'm leaning toward option 1 till we start actually using the
measured_wiresproperty more often.
Thanks so much for your feedback! I've made the property "lazy-loaded". I've also moved the new line in the Changelog to the Improvements section, added my name and addressed the rest of your notes. :)
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 99.66%. Comparing base (
248a808) to head (d0b5ed4).
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3655      +/-   ##
==========================================
- Coverage   99.67%   99.66%   -0.01%     
==========================================
  Files         422      422              
  Lines       40655    40369     -286     
==========================================
- Hits        40522    40235     -287     
- Misses        133      134       +1     
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Just looking for a final review at this stage.
Thanks @comp-phys-marc! I've let the team know that this PR is review ready.
Hello, I have updated and merged in main. I am wondering if there is anything else that needs be done for this PR to be reviewed? All checks are passing @timmysilv @albi3ro. Thank you. :)
Hi @comp-phys-marc! Thanks a lot for opening this PR and returning back to it after a few months.
We've had an internal discussion about the utility of having a measured wires attribute, as requested in the original #3203 issue. Since that issue was opened, things have evolved a bit. In fact, we're moving towards a new program representation - you can check it out here. While QuantumScript will still exist for the foreseeable future, we're cautious about changing it up during this transition period because we'd like to avoid the entropy and additional maintenance burden.
Because of this, we think that it would be best to close #3203 and this PR. Sorry for this, we know that it might be a bit frustrating given the original request in #3203. However, we still love to work with contributors like yourself and could suggest a fresher issue to work on if you would like.
Hello, thanks so much for getting back to me on this! It's no problem, and it's exciting that you are moving to a new program representation. Please do feel free to suggest a fresher issue for me to work on. :)