-
Notifications
You must be signed in to change notification settings - Fork 622
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 hypothesis tests for data operators. #1957
Open
hodgestar
wants to merge
40
commits into
qutip:master
Choose a base branch
from
hodgestar:feature/add-hypothesis-tests-for-data-operators
base: master
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 39 commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
b9bf808
Add hypothesis strategies for qutip data objects.
hodgestar b7aea8b
Add hypothesis test for operators.
hodgestar 2a22493
Ignore hypothesis folder.
hodgestar cec89b9
Add hypothesis to test dependencies.
hodgestar da1ca97
Add assert_allclose and note helpers. Adjust qobj_np strategy to corr…
hodgestar 57f0ee7
Update tests to use new strategy helpers.
hodgestar c85c38c
Add dtype information to notes.
hodgestar 1d74da8
Add utilities for treating infs as nans and silencing numpy numerical…
hodgestar 9a78395
In addition test, ignore arithmetic warnings and treat inf as nans.
hodgestar 48ca9ab
In subtraction test, ignore arithmetic warnings and treat inf as nans.
hodgestar 486b9db
Update remaining tests to strategy helpers and add test for iszero.
hodgestar a5b6120
Fix bug in iszero that caused nan's to be considered zero.
hodgestar 4a4f59c
Remove incorrect fast path in mul_csr.
hodgestar 0e80dbf
Add a strategy for generating suitably shaped datas.
hodgestar 38b4130
Add a test for comparing the equality of differently shaped data.
hodgestar 81a5a64
Use qobj_shaped_datas to correctly specify the shapes of datas for ma…
hodgestar ea6ed3a
Move utilities to the end of the strategies file.
hodgestar 4fc01f2
Add a nicer shared-strategy based helper for generating compatible sh…
hodgestar d55ebd0
Remove debugging print.
hodgestar c4f0d35
Fix atol used for expected result in iszero tests.
hodgestar 67aa5cf
Slightly improve formatting of Qobj and Data notes.
hodgestar 2aee70c
Slightly loosen absolute tolerance on matmul test.
hodgestar 78970fe
Merge branch 'dev.major' into feature/add-hypothesis-tests-for-data-o…
hodgestar 7379dfc
Set tolerance when testing iszero.
hodgestar acf138f
Merge branch 'master' into feature/add-hypothesis-tests-for-data-oper…
hodgestar e0b8d5e
Merge branch 'master' into feature/add-hypothesis-tests-for-data-oper…
hodgestar 888c411
Add missing 'except *' to Data.trace methods.
hodgestar 7ab5dce
Fix typo and remove TODO.
hodgestar b48f786
Add tests for trace, adjoint, conj, transpose and copy.
hodgestar 8de0220
Tighten up some tolerance settings.
hodgestar bda1ed2
Add a strategy specifically for use in testing matmul which is partic…
hodgestar 3394850
Add decorator for asserting that a test raises an exception on specif…
hodgestar ed8d589
Allow CSR scalar multiplication and division to raise exceptions for …
hodgestar 99c9310
Raise errors when multiplying CSR matrices by non-finite scalars.
hodgestar 0313a6f
Add towncrier entry.
hodgestar 1d56725
Add a link to the documentation on Hypothesis' note function.
hodgestar 0835021
Shorten overly long docstring line.
hodgestar 5d6c0b4
Wrap numpy .trace() with ignore_arithmetic_warnings to avoid overflow…
hodgestar cb9f849
Use numpy.zeros(a.shape) instead of numpy.zeros_like(a.to_array()).
hodgestar 133677f
Explicitly set rtol to 0 when determining expected value of equality …
hodgestar 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,3 +43,5 @@ benchmark/benchmark_data.js | |
*-tasks.txt | ||
*compiled_coeff* | ||
result_images/ | ||
|
||
.hypothesis/ |
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 |
---|---|---|
@@ -0,0 +1 @@ | ||
Add hypothesis test for Data class operations (#1957). |
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
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
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
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
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 |
---|---|---|
@@ -0,0 +1,195 @@ | ||
import numpy | ||
|
||
from hypothesis import given, strategies as st | ||
|
||
import qutip | ||
from qutip.core import data as _data | ||
from qutip.tests import strategies as qst | ||
|
||
same_shape = st.shared(qst.qobj_shapes()) | ||
matmul_shape_a, matmul_shape_b = qst.qobj_shared_shapes([ | ||
("n", "k"), | ||
("k", "m"), | ||
]) | ||
|
||
|
||
@given(qst.qobj_np(), qst.qobj_dtypes()) | ||
def test_data_create(np_array, dtype): | ||
data = _data.to(dtype, _data.create(np_array)) | ||
qst.note(data=data, np_array=np_array) | ||
qst.assert_allclose(data.to_array(), np_array) | ||
|
||
|
||
@given(qst.qobj_datas()) | ||
def test_data_neg_operator(data): | ||
neg = -data | ||
qst.note(neg=neg, data=data) | ||
qst.assert_allclose(neg.to_array(), -data.to_array()) | ||
|
||
|
||
@given(qst.qobj_datas()) | ||
def test_data_iszero(data): | ||
result = _data.iszero(data, tol=1e-15) | ||
qst.note(result=result, data=data) | ||
with qst.ignore_arithmetic_warnings(): | ||
np_array = data.to_array() | ||
expected = numpy.allclose( | ||
np_array, numpy.zeros_like(np_array), atol=1e-15, rtol=1e-15, | ||
) | ||
assert result is expected | ||
|
||
|
||
@given(qst.qobj_datas(shape=same_shape), qst.qobj_datas(shape=same_shape)) | ||
def test_data_add_operator(a, b): | ||
result = a + b | ||
qst.note(result=result, a=a, b=b) | ||
with qst.ignore_arithmetic_warnings(): | ||
expected = a.to_array() + b.to_array() | ||
qst.assert_allclose(result.to_array(), expected, treat_inf_as_nan=True) | ||
|
||
|
||
@given(qst.qobj_datas(shape=same_shape), qst.qobj_datas(shape=same_shape)) | ||
def test_data_minus_operator(a, b): | ||
result = a - b | ||
qst.note(result=result, a=a, b=b) | ||
with qst.ignore_arithmetic_warnings(): | ||
expected = a.to_array() - b.to_array() | ||
qst.assert_allclose(result.to_array(), expected, treat_inf_as_nan=True) | ||
|
||
|
||
@given(st.complex_numbers(), qst.qobj_datas(shape=same_shape)) | ||
@qst.raises_when( | ||
ValueError, | ||
"Multiplying CSR matrix by non-finite value {x!r}" | ||
" would generate a dense matrix of NaNs.", | ||
when=lambda a, x: not numpy.isfinite(x) and isinstance(a, _data.CSR), | ||
) | ||
def test_data_scalar_multiplication_left_operator(x, a): | ||
result = x * a | ||
qst.note(result=result, x=x, a=a) | ||
with qst.ignore_arithmetic_warnings(): | ||
expected = x * a.to_array() | ||
qst.assert_allclose(result.to_array(), expected, treat_inf_as_nan=True) | ||
|
||
|
||
@given(qst.qobj_datas(shape=same_shape), st.complex_numbers()) | ||
@qst.raises_when( | ||
ValueError, | ||
"Multiplying CSR matrix by non-finite value {x!r}" | ||
" would generate a dense matrix of NaNs.", | ||
when=lambda a, x: not numpy.isfinite(x) and isinstance(a, _data.CSR), | ||
) | ||
def test_data_scalar_multiplication_right_operator(a, x): | ||
result = a * x | ||
qst.note(result=result, a=a, x=x) | ||
with qst.ignore_arithmetic_warnings(): | ||
expected = a.to_array() * x | ||
qst.assert_allclose(result.to_array(), expected, treat_inf_as_nan=True) | ||
|
||
|
||
@given( | ||
qst.qobj_datas(shape=same_shape), | ||
st.complex_numbers(min_magnitude=1e-12), | ||
) | ||
@qst.raises_when( | ||
ValueError, | ||
"Multiplying CSR matrix by non-finite value {inv_x!r}" | ||
" would generate a dense matrix of NaNs.", | ||
when=lambda a, x: not numpy.isfinite(1 / x) and isinstance(a, _data.CSR), | ||
format_args=lambda a, x: {"inv_x": 1 / x}, | ||
) | ||
def test_data_scalar_division_operator(a, x): | ||
result = a / x | ||
qst.note(result=result, a=a, x=x) | ||
with qst.ignore_arithmetic_warnings(): | ||
expected = a.to_array() * (1 / x) | ||
qst.assert_allclose(result.to_array(), expected, treat_inf_as_nan=True) | ||
|
||
|
||
@given(qst.qobj_datas(shape=same_shape), qst.qobj_datas(shape=same_shape)) | ||
def test_data_equality_operator_same_shapes(a, b): | ||
with qutip.CoreOptions(atol=1e-15): | ||
result = (a == b) | ||
qst.note(result=result, a=a, b=b) | ||
with qst.ignore_arithmetic_warnings(): | ||
expected = numpy.allclose( | ||
numpy.zeros(a.shape), | ||
a.to_array() - b.to_array(), | ||
atol=1e-15 | ||
) | ||
assert result == expected | ||
|
||
|
||
@given(qst.qobj_datas(), qst.qobj_datas()) | ||
def test_data_equality_operator_different_shapes(a, b): | ||
with qutip.CoreOptions(atol=1e-15): | ||
result = (a == b) | ||
qst.note(result=result, a=a, b=b) | ||
if a.shape != b.shape: | ||
assert result is False | ||
else: | ||
with qst.ignore_arithmetic_warnings(): | ||
expected = numpy.allclose( | ||
numpy.zeros(a.shape), | ||
a.to_array() - b.to_array(), | ||
atol=1e-15 | ||
) | ||
assert result == expected | ||
|
||
|
||
@given( | ||
qst.qobj_datas_matmul(shape=matmul_shape_a), | ||
qst.qobj_datas_matmul(shape=matmul_shape_b), | ||
) | ||
def test_data_matmul_operator(a, b): | ||
result = a @ b | ||
qst.note(result=result, a=a, b=b) | ||
with qst.ignore_arithmetic_warnings(): | ||
expected = a.to_array() @ b.to_array() | ||
qst.assert_allclose( | ||
result.to_array(), expected, | ||
atol=1e-10, rtol=1e-10, treat_inf_as_nan=True, | ||
) | ||
|
||
|
||
@given(qst.qobj_datas()) | ||
@qst.raises_when( | ||
ValueError, | ||
"matrix shape {shape} is not square.", | ||
when=lambda data: data.shape[0] != data.shape[1], | ||
format_args=lambda data: {"shape": data.shape}, | ||
) | ||
def test_trace(data): | ||
result = data.trace() | ||
with qst.ignore_arithmetic_warnings(): | ||
expected = data.to_array().trace() | ||
qst.note(result=result, data=data) | ||
qst.assert_allclose(result, expected) | ||
|
||
|
||
@given(qst.qobj_datas()) | ||
def test_adjoint(data): | ||
result = data.adjoint() | ||
qst.note(result=result, data=data) | ||
qst.assert_allclose(result.to_array(), data.to_array().T.conj()) | ||
|
||
|
||
@given(qst.qobj_datas()) | ||
def test_conj(data): | ||
result = data.conj() | ||
qst.note(result=result, data=data) | ||
qst.assert_allclose(result.to_array(), data.to_array().conj()) | ||
|
||
|
||
@given(qst.qobj_datas()) | ||
def test_transpose(data): | ||
result = data.transpose() | ||
qst.note(result=result, data=data) | ||
qst.assert_allclose(result.to_array(), data.to_array().T) | ||
|
||
|
||
@given(qst.qobj_datas()) | ||
def test_copy(data): | ||
result = data.copy() | ||
qst.note(result=result, data=data) | ||
qst.assert_allclose(result.to_array(), data.to_array()) |
Oops, something went wrong.
Oops, something went wrong.
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.
I hate the removal of this shortcut.
mul(data, 0)
can happen often in QobjEvo when the coefficient is a pulse or step function.Without this shortcut, they need to loop over the values twice, once for this
mul
, once for theadd
ormatmul
following.Thus this shortcut will actually help users.
It is also one of the only places where there is a
mul(data, 0)
in the code.In some theoretical case, it can shallow a NaN, but I don't see any realistic case where this could hide an error.
So everyone loses for a case that will probably never happen...
Can you write a code example where this would save us? Something that looks like it should work, but cause infinities or NaN that would be erased by a multiplication by 0 without returning garbage.
If we can't even think of such a case, let's keep the shortcut.
If you can find one, let's loop over the values to find Nans and return the zeros crs if none are found so we at least loop only once over the values.
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.
Maybe we just need to make
_data.zeros[type(x)](*x.shape)
easier to use so that we don't need to writemul(data, 0)
? I can add_data.zeros_like(x)
if that would be better?In the specific case where
mul(data, 0)
is intended to create a matrix of zeros of the right shape, the current fast path can never be wrong since that is exactly what it does. In general,mul(data, 0)
is currently just incorrect floating point arithmetic.I was wondering about adding
has_non_finite
flag to CSRs. It will be a bit of pain to keep track of everywhere but it would allow us to fast path the all finite cases without sacrificing correctness.If this is a blocker for this PR, I'm happy to undo the current change and just make the tests pass by just explicitly stating that the expected result is
0
in those cases in the test.I also don't want to sacrifice performance, but it is important to have a clear set of tests that show how we expect the data layer to behave in these edge cases.
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.
I am more thinking about the use in
QobjEvo
. I don't think we ever use it as_data.zeros_like
, in the code base, but that function would be useful, as would be anidentity_like
as we discussed somewhere.I would be fine with a
has_non_finite
flag, but I think we should raise an error anytime we would raise that flag.I think that we could do the check when we tidyup: we already check the scale of all values and it's done in most operation that could create zeros, thus functions at risk.
I see a CSR matrix full of zeros as wrong, if not an error, so this is a blocker to me.
This and tests should pass reliably.
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.
I am using
mul
as_data.zeros_like
in stochastic...