make compatible with TensorOperations v5 #14
Conversation
| - '1.8' # lowest TensorOperations version | ||
| - '1.10' # LTS version | ||
| - '1' # automatically expands to the latest stable 1.x release of Julia |
There was a problem hiding this comment.
I think the recommended way is now:
- 'lts'
- '1'
- '1.8
src/tensoroperations.jl
Outdated
| (N == length(pA) && TupleTools.isperm(pA)) || | ||
| throw(ArgumentError("Invalid permutation of length $N: $pA")) | ||
| size(C) == TupleTools.getindices(size(A), pA) || | ||
| throw(DimensionMismatch("non-matching sizes while adding arrays")) |
There was a problem hiding this comment.
| (N == length(pA) && TupleTools.isperm(pA)) || | |
| throw(ArgumentError("Invalid permutation of length $N: $pA")) | |
| size(C) == TupleTools.getindices(size(A), pA) || | |
| throw(DimensionMismatch("non-matching sizes while adding arrays")) | |
| argcheck_tensoradd(C, A, pA) |
There was a problem hiding this comment.
This only works after making TensorOperations a dependency, right? Also, I currently do not yet use Index2Tuple in the arguments for these methods. I will first study your PR on top of this one.
src/tensoroperations.jl
Outdated
| NC == length(p) || | ||
| throw(ArgumentError("Invalid selection of $NC out of $NA: $p")) | ||
| NA - NC == 2 * length(q1) == 2 * length(q2) || | ||
| throw(ArgumentError("invalid number of trace dimension")) |
There was a problem hiding this comment.
| NC == length(p) || | |
| throw(ArgumentError("Invalid selection of $NC out of $NA: $p")) | |
| NA - NC == 2 * length(q1) == 2 * length(q2) || | |
| throw(ArgumentError("invalid number of trace dimension")) | |
| argcheck_tensortrace(C, A, p, q) |
| dense_result = TensorOperations.ncon(Array.(tensors), indices, conjlist) | ||
|
|
||
| @test Array(sparse_result) ≈ dense_result | ||
| if SparseArrayKit.nonzero_length(sparse_result) > 0 |
There was a problem hiding this comment.
Does this not work if the result is completely sparse?
There was a problem hiding this comment.
It does, but the tests take quite some time, my impression being that it is the dense tensor contraction taking most of the time. And since quite a few of the tests actually yield a zero tensor, I thought to not contract these in the dense case to save on compute time. The resulting test is still meaningful, but I would hope that the tests leading to nonzero tensors would be sufficient to check for errors.
There was a problem hiding this comment.
Maybe just a comment for future reference would be helpful then :)
* use new lts syntax * Reabsorb TensorOperations as full dependency * Fix Aqua compat * Add missing Compat entries
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #14 +/- ##
==========================================
+ Coverage 89.88% 90.07% +0.19%
==========================================
Files 8 7 -1
Lines 425 413 -12
==========================================
- Hits 382 372 -10
+ Misses 43 41 -2 ☔ View full report in Codecov by Sentry. |
2 participants
No description provided.