[WIP] some einsum tests fail #391
Merged
Conversation
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
|
I guess 0 are technically random numbers. I'll check it out
…On Wed, Mar 1, 2023, 7:08 PM Eduard Valeyev ***@***.***> wrote:
@asadchev <https://github.com/asadchev> some unit tests actually used
zero-filled tensors (due to bug in fill_random for tensors of integers) and
thus "passed" but should not have. I fixed the fill_random so you can see
which tests fail. Interesting not all TA-based einsum tests fail, on my
machine I see this:
/Users/efv/code/tiledarray/tests/einsum.cpp:970: error: in "einsum_tiledarray/einsum_tiledarray_hi_hi_h": check isApprox(result, reference) has failed
/Users/efv/code/tiledarray/tests/einsum.cpp:970: error: in "einsum_tiledarray/einsum_tiledarray_hji_jih_hj": check isApprox(result, reference) has failed
/Users/efv/code/tiledarray/tests/einsum.cpp:970: error: in "einsum_tiledarray/einsum_tiledarray_replicated": check isApprox(result, reference) has failed
/Users/efv/code/tiledarray/tests/einsum.cpp:970: error: in "einsum_tiledarray/einsum_tiledarray_replicated": check isApprox(result, reference) has failed
------------------------------
You can view, comment on, or merge this pull request online at:
#391
Commit Summary
- 47e503f
<47e503f>
[unit] einsum tests comparison for real numbers should not be bitwise
- 4f3aaaf
<4f3aaaf>
fixup DistArray::fill_random
File Changes
(2 files <https://github.com/ValeevGroup/tiledarray/pull/391/files>)
- *M* src/TiledArray/util/random.h
<https://github.com/ValeevGroup/tiledarray/pull/391/files#diff-f2e34a4b81bf3a5799a331f1e381bf289d8beb8a724ce7b3051a9fdfa4cfa63e>
(72)
- *M* tests/einsum.cpp
<https://github.com/ValeevGroup/tiledarray/pull/391/files#diff-05de02c9e0f260ce7e0bc399f2b6f8b0d20a73fd8d6d22cef11112460c9024f9>
(24)
Patch Links:
- https://github.com/ValeevGroup/tiledarray/pull/391.patch
- https://github.com/ValeevGroup/tiledarray/pull/391.diff
—
Reply to this email directly, view it on GitHub
<#391>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABH62KDJOTLOYB75TQ7WTITWZ7QJDANCNFSM6AAAAAAVMWSIWE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
amends 6417297 to make MakeRandom<T> generate non-zeroes for integer T
evaleev
force-pushed
the
evaleev/fix/einsum
branch
from
4f3aaaf to
e2009bf
Compare
|
the issue was in the "Hadamard reduction" branch of the code |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
@asadchev some unit tests actually used zero-filled tensors (due to bug in fill_random for tensors of integers) and thus "passed" but should not have. I fixed the fill_random so you can see which tests fail. Interesting not all TA-based einsum tests fail, on my machine I see this: