Jutho / TensorOperations.jl

Julia package for tensor contractions and related operations
https://jutho.github.io/TensorOperations.jl/stable/
Other
438 stars 55 forks source link

Add tests `indices` and `labels` #159

Closed lkdvos closed 6 months ago

lkdvos commented 6 months ago

Some simple tests to improve test coverage

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (1a19cd2) 82.00% compared to head (aa342d9) 82.69%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #159 +/- ## ========================================== + Coverage 82.00% 82.69% +0.69% ========================================== Files 25 25 Lines 2150 2150 ========================================== + Hits 1763 1778 +15 + Misses 387 372 -15 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Jutho commented 6 months ago

Thanks. In principle floating point numbers are not allowed as labels:

julia> TensorOperations.isindex(3.0)
false

but clearly this is not checked by the add_indices function 😄 . Can I merge or do you want to change the test in order not to confuse people?

lkdvos commented 6 months ago

I've changed the tests to improve clarity, although now I am wondering if these functions should assert that the labels are valid? I think @asserts are disabled when optimisation is set to high, so this should not impact performance? [edit] currently @assert only states in its documentation that it may be optimized away, but I don't think this actually happens. With that in mind, probably the best option is to merge this as-is, and not do actual checking in these methods. I don't think there is enough use to warrant a rewrite in terms of unsafe_ variants.