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 new method for single bitstring target when measuring stabilizerstate probabilities_dict #12147

Conversation

DerekMiner
Copy link
Contributor

@DerekMiner DerekMiner commented Apr 5, 2024

fixes #11425

Allows a user to pass a single bitstring target value to get the probability value for. In cases where you have a large amount of qubits, this would take a very long time to retrieve the probability value for the single bitstring via the probabilities_dict method as it calculates all probabilities. A new method was added called probabilities_dict_from_bitstring, that allows the user to pass a single bitstring target value to retrieve a value for.
This allows using a large amount of qubits with a very quick retrieval time for the required value as it no longer has to calculate all the possibly probabilities.

According to @ShellyGarion

The main use-case that I can think of is of an n-qubit StabilizerState when n is large (say, n=100). In this case, the StabilizerState is quadratic in the number of qubits, while obtaining all the outcomes can potentially grow in some cases as 2^n (e.g. where there is an H gate on all qubits). Hence, it makes sense to take only one outcome, or several outcomes whose number grows at most polynomially in n (it doesn't make sense to take half the 2^n outcomes for example).

and @ginsparg

For a variety of purposes (including classical shadows), it is necessary to calculate only a single probability for some target bitstring via .get_probability(target)

If a user wants to retrieve multiple bitstring targets they can call the method multiple times for now, there doesn't appear to be a current use case for passing in multiple bitstrings at a time. This could easily be added without much effort, without use of caching support. Caching could be used if there was ever a use case for a large amount of bitstrings to be calculated to improve performance, but less then all possible bitstrings.

As suggested by @ShellyGarion I removed the performance measuring code that checks that it is faster to perform only a single measurement. This should be self apparent as performing tests with 200 qubits would possible take up to 2^200 outcomes (1.606938e+60) and likely wouldn't finish calculating these within our lifetime.

When I did have this performance measuring code in from the previous PR #11947, there was a huge performance increase measured with the checks I performed, roughly close to 1/amount_of_possible_outcomes of time, with a tiny bit of overhead from caching (but improved from not caching).

added new method probabilities_dict_from_bitstrings, moved logic from probabilities_dict to probabilities_dict_from_bitstrings with ability to pass target, probabilities_dict will pass a target of None.
…bability for qubit

Added some timing of the tests to determine if the version with a specfic target runs faster then the non-targetted version (which must calculate all 2^n possibilities for stabilizer state test_probablities_dict_single_qubit. When given a target it can skip branches that were not wanting to be calculated
simplified the method probabilities_dict_from_bitstrings, less checking was needed, simpler handling of no target value passed in
In stabilizerstate, added the ability to cache previously calculated branches so they do not get recalculated when attempting to get target values, gives a better starting point to calculate from instead of starting from an unknown probability value of 1.0
when creating an array for the outcome in probability, it is only built if the cache key is not found
…disabling cache

added more optimization for performance when performing calculations for branches using caching and targets, it will now also store the deterministic node values which gives a very slight performance increase, migrated to most efficient ways to store and retrieve cache values. The performance increase when using a large amount of qubits is huge, when using a very small amount of qubits the very slight over head of caching makes it about the same. Increasing test coverage for using targets with probabilities
implemented more accurate tests that verify probability caching
for some calculations the cached results were in the wrong place which could cause false results for certain test cases with 3+ qubits
overhead was too high compared to calculating
Fixed failing test that would randomly occur, the stabilizer state was not being cached as well, which would sometimes produce a failure. After caching the object to restore when calculating starting at a node down the branch, this failure no longer occurs. Need to optimize and clean up the code a bit, but functioning
@DerekMiner DerekMiner changed the title [WIP] Add new method for single bitstring target when measuring stabilizerstate probabilities_dict Add new method for single bitstring target when measuring stabilizerstate probabilities_dict Apr 18, 2024
Copy link
Member

@ShellyGarion ShellyGarion left a comment

Choose a reason for hiding this comment

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

Thank you very much for this high quality PR @DerekMiner !
I only have a few minor comments.

DerekMiner and others added 11 commits April 25, 2024 09:07
…ng' of https://github.com/DerekMiner/QisKit into add_target_stabilizerstate_probabilities_single_bitstring
moved the helper function _get_probabilities_dict to the helper function area next to _get_probabilities

Qiskit#12147 (comment)
added a test that does not have all equal probabilities and is not a GHZ state

Qiskit#12147 (comment)
combine the large and medium tests into one due to taking too long to run the tests and lowered the amount of qubits
removed redundant test in test_probabilities_dict_medium_num_qubits, as this was combined with the large test
renamed test_probabilities_dict_medium_num_qubits in test_stabilizerstate to test_probabilities_dict_from_bitstring
Copy link
Member

@ShellyGarion ShellyGarion left a comment

Choose a reason for hiding this comment

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

Looks great now, thanks for the efforts!
I only found last minor misprint.

qiskit/quantum_info/states/stabilizerstate.py Outdated Show resolved Hide resolved
@ShellyGarion ShellyGarion added Changelog: New Feature Include in the "Added" section of the changelog mod: quantum info Related to the Quantum Info module (States & Operators) labels May 8, 2024
typo for description for word targetting, changed to targeting
@ShellyGarion ShellyGarion self-requested a review May 8, 2024 14:41
Copy link
Member

@ShellyGarion ShellyGarion left a comment

Choose a reason for hiding this comment

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

Nice work! Thanks for your contribution to Qiskit!

@ShellyGarion ShellyGarion added this pull request to the merge queue May 8, 2024
Merged via the queue into Qiskit:main with commit 87c14cb May 8, 2024
15 checks passed
ElePT pushed a commit to ElePT/qiskit that referenced this pull request May 31, 2024
…tate probabilities_dict (Qiskit#12147)

* added new method, deterministic helper unction

added new method probabilities_dict_from_bitstrings, moved logic from probabilities_dict to probabilities_dict_from_bitstrings with ability to pass target, probabilities_dict will pass a target of None.

* Adding test timing, fixed algorithm for determining if should get probability for qubit

Added some timing of the tests to determine if the version with a specfic target runs faster then the non-targetted version (which must calculate all 2^n possibilities for stabilizer state test_probablities_dict_single_qubit. When given a target it can skip branches that were not wanting to be calculated

* Adding more tests, fixing deterministic issues with bad target

* Prob of targets with 0.0 probability

* target calc changes

* Simpler way to get all targets

* Need to improve performance with multiple targets

* Simplified the probabilities_dict_from_bitstrings method

simplified the method probabilities_dict_from_bitstrings, less checking was needed, simpler handling of no target value passed in

* Adding tests for targets

* Added tests to test_probablities_dict_qubits

* Add performance boost check when using targets for test_probablities_dict_qubits

* Added Caching to prevent duplicate branch calculations

In stabilizerstate, added the ability to cache previously calculated branches so they do not get recalculated when attempting to get target values, gives a better starting point to calculate from instead of starting from an unknown probability value of 1.0

* improve performance by performing expensive operations only when needed

when creating an array for the outcome in probability, it is only built if the cache key is not found

* Optimization for caching, deterministic calcs for branches, enabling/disabling cache

added more optimization for performance when performing calculations for branches using caching and targets, it will now also store the deterministic node values which gives a very slight performance increase, migrated to most efficient ways to store and retrieve cache values. The performance increase when using a large amount of qubits is huge, when using a very small amount of qubits the very slight over head of caching makes it about the same. Increasing test coverage for using targets with probabilities

* improved tests, checking of caching vs not caching

implemented more accurate tests that verify probability caching

* added more tests and variations for stabilizerstate

* Corrected deterministic probability calculations

* Fixed bug with cache in wrong place for some calculations

for some calculations the cached results were in the wrong place which could cause false results for certain test cases with 3+ qubits

* added more test cases

* Probabilities update for moved method call

* Added test for random test_probs_random_subsystem

* fixed single probabilities tests lost in previous merge

* Removed caching of deterministic values

overhead was too high compared to calculating

* Fixed failing test with test_probs_random_subsystem

Fixed failing test that would randomly occur, the stabilizer state was not being cached as well, which would sometimes produce a failure. After caching the object to restore when calculating starting at a node down the branch, this failure no longer occurs. Need to optimize and clean up the code a bit, but functioning

* Adding tests back that were not test_probs_random_subsystem, removed to test

* Created ProbabilityCache object for dealing with caching

* Finished adding ProbabilityCache object and commenting

* Added more documentation for stabilizer state

* Fixing style with tox -eblack

* commenting fixes in stabilizerstate

* Added release note

* Updated release notes

* added __future__ to probabilitiescache, removed unused import stabilizerstate

* run tox -eblack to correct probabilitiescache

* Corrected lint issues in multiple classes

fixed lint issues in test_probabilitiycache, test_stabilizerstate, stabilizerstate, and probabilitycache

* fixed more pylint lint issues

pylint -rn qiskit test tools
Corrected in probabilitycache, stabilizerstate, test_probabilitycach, test_stabilizerstate

* fixed more lint issues

fixed with:
tox -eblack

verified with:
black --check qiskit test tools examples setup.py
pylint -rn qiskit test tools
tox -elint

* added a bit more variance allowance for performance tests

renamed test_probabilitycache.py file to correct spelling
added a bit more performance_varability_percent percent time allowed from 0.001 to 0.005

* Corrected Lint issues, added test for probabilitycache

added test_probabilitycache test, fixed lint issues and type issues in stabilizerstate, test_stabilizerstate

* Fixed remaining import ordering issue

* readded ddt decorator for test_probabilitycache

* Changed to using process_time_ns from monotonic for performance measuring

* Added output when performance times fail to aid in debugging in test_stabilizerstate

* moved performance end time out of same method to calculate times

* changed method name in test_stabilizerstate

changed probability_percent_of_calculated_branches to _probability_percent_of_calculated_branches

* Added GC disabling between timed tests

* Attempting using perf_counter_ns instead of process_time_ns

* Updated commenting, moved 'X' outcome check

moved 'X' in outcome out of the for loop for StabilizerState._get_probabilities to help performance

* more comment updating

* Changed performance timer based on OS type

* Changing to time.thread_time_ns(), and raising performance varabilitiy by 0.005

using time.thread_time_ns() for benchmarking instead of time.perf_counter_ns() and time.process_time_ns() based on the OS in test_stabilizerstate. It seems that time is being counted that shouldn't be with the other methods that causes false reports of the work taking longer then it does. Raising the performance_varability_percent in test_stablilizerstate by 0.005 to 0.01 for more headroom for performance checking

* changing time.thread_time() vs time.thread_time_ns() to see if compatible with windows

also added one more target for test_4 to differentiate no caching against caching in test_stabilizerstate

* Changing performance timer based on OS for test_stablizerstate

perf_counter_ns for win32 and thread_time for all other OS

* fixed small issue in probabilitycache, improved probabilitycache test

fixed issue when checking the cache key that was not operating correctly, internally the key is always a str, but when a list[str] was passed in the check was not correct, so changed to check if an instance of a list, and will join the outcome val to a str for the internal cache key. Improved the probabilitycache to test having alternating keys of str and list[str] to test this internal key checking

* Added correct type hinting to probabilitycache

* removed typing hint module

removed the use of typing module for type hinting, looks like python wants to get away from using this module for type hinting, also fixed some commenting issues in stabilizerstate, test_probabilitycache, test_stabilizerstate, and probabilitycache files

* Fix test pylint failure of Value 'list' is unsubscriptable (unsubscriptable-object)

fixing tests failing with pylint when lint checking is performed. This worked locally and wasn't failing, but when it runs in the CI/CD process it fails, so adding from __future__ import annotations to avoid this failure for test_probabilitycache and test_stabilizerstate as new tests include type hinting

* Fix small comment error in test, Added use_caching to tests, simplified cache calls

Fixed a small error in my commenting for the test_stabilizerstate.py and forced use_caching parm in the call of probabilities_dict_from_bitstrings in the tests, even tho it is used as default, to make sure it is clear in the tests. Simplfied the adding values to Probabilitycache, and simplified the code in StabilizerState._get_probabilities() setting values in cache and retrieving the state when using cache, made cache_key in probabilitycache "private" and changed method to _cache_key

* Significant code refactoring in StabiizerState

Significant code refactoring in stabilizerstate. I was able to condense the use the cache to a smaller area in the _get_probabilities helper method so it is easier to work with and more clear. I removed methods I created that were single use helper methods that were able to be condensed down into simpler code such as _branches_to_measure, _retrieve_deterministic_probability, _branches_to_measure, _is_qubit_deterministic. Since these are 1 time use methods it didn't make sense to keep them around. The _get_probabilities helper method is much simpler and clearer, closer to the original with less tweaks and functioning with the same performance, and more clarity.  Most of the code changed at this point is adding tests, most of this commit was removing unnecessary code and refactoring

* Fixed stabilizerstate cache key outcome not found

there was a check to make sure a key could be found before that was removed in the refactoring, once and awhile you get a target that can't be found in the cache because of the algorithm of looking for a key by increasing the number of X from the left to the right. There are situations where the X would be in the middle such as 0X1 that won't be found. The algorithm in the probability cache could be changed to use BST which would very rarely help performance, the fix is to make sure a key can be found and then only use the cache if that is the case

* Moved all caching retrieval outside of _get_probabilities for performance

there is a slight overhead when recursively iterating through the _get_probabilities helper when having to check if caching should be retrieved. Realistically when you use caching it is just to get you a better starting point, so it only needs to be retrieved once. The cache inserting remains in the _get_probabilities helper to build up the cache, but the logic was significantly simplified in the _get_probabilities helper, and now only a 1 time cache retrieval is performed for each target via the probabilities_dict_from_bitstrings method

* Simplified Bitstring method for probability cache, removed unnecessary classes

Removed Probabilitycache class, test_probabilitiycache.py. Simplified test_stabilizerstate.py for new bitstring probability testing methods. changed method probabilities_dict_from_bitstrings to probabilities_dict_from_bitstring, and simplified by removing cache, and only allowing a single string value to be passed in

* Commenting update for _probabilities_bitstring_verify

updating the commenting for _probabilities_bitstring_verify, move to top of file

* black lint formatter ran

* Uncomment out line causing circular import

when running locally I get a circular import for two_qubit_decompose.py on line 154 "_specializations = two_qubit_decompose.Specialization", I need to comment out to test locally, but did not mean to commit this.

Simplified the stablizierstate self._get_probabilities method but removing an unnecessary check

* Remove type hints from test_stabilizerstate

* Updated test_stabilizerstate, removed helper test method, new release note

update the tests in test_stabilizerstate to no longer use a helper method for checking all the bitstring measurements in. Created a _get_probabilities_dict helper method in stabilizer state to be used for the probabilities_dict and probabilities_dict_with_bitstring methods. This was needed so that they could share the logic of the measurements, but also so that you could enforce in the probabilities_dict_with_bitstring method that outcome_bitstring must be provided. Previouslly this was optional and the user could avoid passing in a value, but this would essentially make it function the same as probabilities_dict. Now with the helper it enforces they must provide a bitstring

* Added large qubit test with h gates, reorder imports for test_stabilizerstate

added new test method test_probabilities_dict_large_num_qubits with large amount of qubit tests using an outcome bitstring target, num_qubits=[25, 50, 100, 200, 300, 400, 500, 600, 750], normally get any results for 750 qubits would require ~5.922387e+225 full bitstring calculations, but if you want one target bitstring you can perform ~750 measurements for a single probability for one result and get the value quickly

* moved methods out of loop that are unnecessary in test_stabilizerstate

moded target dict calculation out of loop that is not necessary to recalculate through each sample loop calculation

* broke out medium hgate test to own test method, issue with assertTrue type in test

testing a variety of hgates that also do the full probabilities_dict calculations up to 9 qubits called test_probabilities_dict_hgate_medium_num_qubits, also changed a assertTrue typo to assertDictEqual in the test_probabilities_dict_large_num_qubits method

* fixed commenting in stabilizerstate, removed unnecessary outcome_bitstring array access syntax

fixed some commenting for probabilities_dict_from_bitstring, and removed unnecessary syntax for accessing the array, was outcome_bitstring[i : i + 1] now outcome_bitstring[i] in _get_probabilities

* Condensed tests in test_stabilizerstate, added helper test class, more test coverage

added class StabilizerStateTestingTools in file test_stabilizerstate, these methods needed to be moved outside of the TestStabilizerState class because they are also needed in the TestStabilizerStateExpectationValue class within the file. It did not make sense to put these helper methods in the QiskitTestCase class, or the BaseQiskitTestCase class as these helpers are specific to helping test a new method in the stabilizerstate class.

I condensed redudnant code for checking the individual bitstrings into helper method _verify_individual_bitstrings which takes a target dict, uses each entry in the dict to attempt to get the single probability, and verify each, which drastically raises the test coverage of this new method probabilities_dict_from_bitstring.

added helper method StabilizerStateTestingTools._bitstring_product which builds a dict of the product of 0, 1 for a lenth passed in, used in several test cases for testing the zero probabilities.

In the test cases you will also notice cases where I update the target dict target.update(StabilizerStateTestingTools._bitstring_product( num_qubits, target)), which builds the rest of the bitstring values that will have a 0 probability, which is then passed into the _verify_individual_bitstrings method to verify they get the correct 0 value

I reduced the number of qubits for the test_probabilities_dict_hgate_large_num_qubits from tests of num_qubits=[25, 50, 100, 200, 300, 400, 500, 600, 750] to num_qubits=[25, 50, 100, 200] due to memory issues on windows, and the largest use case I have received so far is 100 qubits, so going above that by 2x

Overall test coverage should be dramatically increased now testing for 0 probabilities, checking every bitstring individually has a probability, and simplifying the code, removing a lot of repetative logic, moved to the helper methods

* Fixed isues in release notes

Qiskit#12147 (comment)

Qiskit#12147 (comment)

* Corrected Targetted to targeted in stablizerstate

* Updated commenting about target

Qiskit#12147 (comment)

Qiskit#12147 (comment)

* Moved helper function _get_probabilities_dict

moved the helper function _get_probabilities_dict to the helper function area next to _get_probabilities

Qiskit#12147 (comment)

* differentiated description for probabilities

Qiskit#12147 (comment)

* Added test to test_stabilizerstate

added a test that does not have all equal probabilities and is not a GHZ state

Qiskit#12147 (comment)

* combined 2 test methods

combine the large and medium tests into one due to taking too long to run the tests and lowered the amount of qubits

* removed unnecessary test

removed redundant test in test_probabilities_dict_medium_num_qubits, as this was combined with the large test

* fixed lint issues, renamed test method

renamed test_probabilities_dict_medium_num_qubits in test_stabilizerstate to test_probabilities_dict_from_bitstring

* fixed type for targetting in stabilizerstate

typo for description for word targetting, changed to targeting

---------

Co-authored-by: Shelly Garion <46566946+ShellyGarion@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog Community PR PRs from contributors that are not 'members' of the Qiskit repo mod: quantum info Related to the Quantum Info module (States & Operators)
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

qiskit.quantum_info.StabilizerState needs get_probability() method
4 participants