Trivial tensors fast path into TensorOperations machinery#463
Conversation
Codecov Report❌ Patch coverage is
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
| # the fusiontree machinery and act directly on the `t[]` array view. | ||
| _has_dense_backing(::AbstractTensorMap) = false | ||
| _has_dense_backing(t::TensorMap) = sectortype(t) === Trivial | ||
| _has_dense_backing(t::AdjointTensorMap) = _has_dense_backing(parent(t)) |
There was a problem hiding this comment.
Not a great fan of the name/terminology "backing". Do you expect to have relevant cases where this is not simply equivalent to sectortype(t) === Trivial. A call to _has_dense_backing(t) is always followed by t[], and that getindex method is only defined for tensors with sectortype(t) == Trivial.
There was a problem hiding this comment.
I'm happy to change the name to whatever, but the use case is blocktensors, for which this is not really what I wanted to implement, even though I definitely could if that is where we want to go, but it would be a reimplementation of the permute and then mul calls
There was a problem hiding this comment.
Ok I see. I just don't like introducing a new terminology ("backing") that isn't really well defined. We have storage, but currently all storage is dense. What does it amount to in the blocktensor case? Is it also equivalent to sectortype(t) === Trivial but sectortype is not defined? Or is it at the block level that you want to directly lower to a TensorOperations call, but then the individual blocks can still be nontrivial tensors?
There was a problem hiding this comment.
The problem is just that in this case t[] still works, but now produces a single block(sparse) array with dense blocks, for which I don't have tensor contraction defined. How about has_contiguous_storage?
The alternative is to simply use dispatch and define TrivialTensorMap again, but I decided against that since that gets a little messy with adjointtensormap, for which this also works
There was a problem hiding this comment.
Ok, after some more (AI-assisted) thinking, I will propose: has_array_view. Given that this is a completely internal function that is just there to avoid having this behavior by default for AbstractTensorMap, I don't think it matters too much and I can't come up with too many better alternatives.
There was a problem hiding this comment.
Ok, has_array_view sounds quite good and clear to me.
2 participants
This bypasses some of the unnecessary machinery for dense trivial tensors to hopefully improve compilation times by being able to re-use some of the precompiled things in TensorOperations.
I also found an
allocatorargument not being passed on to thepermutecall, so that should now also be fixed.