-
Notifications
You must be signed in to change notification settings - Fork 984
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
T1 constant calculation #4290
Merged
Merged
T1 constant calculation #4290
Changes from 5 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
6786e2a
Added t1 constant calculation and associated tests, can also plot cur…
anasofiauzsoy df5b532
modified plotting test to include curve fit
anasofiauzsoy b4e5d29
updated tests to plot curve fit with better data
anasofiauzsoy dc1ef40
improved tests for plotting curve fit
anasofiauzsoy 1a206fc
fixed the smaller changes Michael suggested
anasofiauzsoy b8ae3ae
added parametrization and exponential decay test case, also changed t…
anasofiauzsoy bfbbf14
removed surplus warning
anasofiauzsoy 459e352
changed return value to 0 in case of bad fit so it will throw an erro…
anasofiauzsoy 8377c4c
changed to return np.nan if constant cannot be found
anasofiauzsoy 7acd991
changed test name to indicate warning
anasofiauzsoy 8464ead
Merge branch 'master' into t1_decay_4264
anasofiauzsoy 5417ee4
Merge branch 'master' into t1_decay_4264
CirqBot File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,8 @@ | |
|
||
import cirq | ||
|
||
import numpy as np | ||
|
||
|
||
def test_init_result(): | ||
data = pd.DataFrame( | ||
|
@@ -165,6 +167,72 @@ def test_all_off_results(): | |
) | ||
|
||
|
||
def test_constant(): | ||
result_100 = cirq.experiments.T1DecayResult( | ||
data=pd.DataFrame( | ||
columns=['delay_ns', 'false_count', 'true_count'], | ||
index=range(4), | ||
data=[ | ||
[100.0, 6, 4], | ||
[400.0, 10, 0], | ||
[700.0, 10, 0], | ||
[1000.0, 10, 0], | ||
], | ||
) | ||
) | ||
assert np.isclose(result_100.constant, 100, 5) | ||
|
||
result_400 = cirq.experiments.T1DecayResult( | ||
data=pd.DataFrame( | ||
columns=['delay_ns', 'false_count', 'true_count'], | ||
index=range(4), | ||
data=[ | ||
[100.0, 0, 10], | ||
[400.0, 6, 4], | ||
[700.0, 10, 0], | ||
[1000.0, 10, 0], | ||
], | ||
) | ||
) | ||
assert np.isclose(result_400.constant, 400, 5) | ||
|
||
MichaelBroughton marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
def test_curve_fit_plot(): | ||
good_fit = cirq.experiments.T1DecayResult( | ||
data=pd.DataFrame( | ||
columns=['delay_ns', 'false_count', 'true_count'], | ||
index=range(4), | ||
data=[ | ||
[100.0, 6, 4], | ||
[400.0, 10, 0], | ||
[700.0, 10, 0], | ||
[1000.0, 10, 0], | ||
], | ||
) | ||
) | ||
|
||
good_fit.plot(include_fit=True) | ||
|
||
bad_fit = cirq.experiments.T1DecayResult( | ||
data=pd.DataFrame( | ||
columns=['delay_ns', 'false_count', 'true_count'], | ||
index=range(4), | ||
data=[ | ||
[100.0, 10, 0], | ||
[400.0, 10, 0], | ||
[700.0, 10, 0], | ||
[1000.0, 10, 0], | ||
], | ||
) | ||
) | ||
|
||
try: | ||
bad_fit.plot(include_fit=True) | ||
assert False | ||
except RuntimeWarning as warning: | ||
assert warning is not None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be a good idea to split this error case off into a separate test method. You can also make use of with pytest.raises(..., match='whatever the error string is'):
... something that breaks... to verify an error gets raised in the test. |
||
|
||
|
||
def test_bad_args(): | ||
with pytest.raises(ValueError, match='repetitions <= 0'): | ||
_ = cirq.experiments.t1_decay( | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be worth looking into parameterizing this test with
@pytest.mark.parameterize
so that you can remove some of the code duplication between the 100 and 400 cases, and just turn the relevant values between these two results into parameters for the test. Here's a quick example: https://github.com/quantumlib/Cirq/blob/master/cirq-core/cirq/ops/common_gates_test.py#L60There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this suggestion! I've added this to the test, along with the explicitly exponential data from your other comment.