-
Notifications
You must be signed in to change notification settings - Fork 19
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
Tensor::rank and contiguous at compile time #44
Comments
Do you mean you want Tensor/TensorView to be able to report this at compile time? rank() is easy, you need to use RangeNd whose Index is std::array (or similar), it's rank() is a static constant then (in principle ... the code right now does not use constexpr). Contiguous() will be hard. |
Yes. Each of them would allow us to have one less if statement... It would be nice if the standard says something about it - otherwise I cannot assume that range can report rank, for instance. |
(I am implementing contract today) |
Do you mean to make TWG spec say that it's possible for rank() to be On Mon, Jun 23, 2014 at 8:54 AM, Toru Shiozaki [email protected]
web: valeyev.net |
Yes. And contiguous() if possible. I suspect that there is some problem in doing that, but wanted to discuss. Otherwise I just introduce a bunch of if statements in the main contract function (for me it's fine, but not sure if it is the right way) |
…:Range spec). Added test to text.C, but only clang++ accepts it. @shiozaki please check with your g++.
It appears to me that we should compute the rank using the type of lobound_ instead of lobound_ itself? I am not sure at all if this is standard compliant. @evaleev Do you know if this is a problem with a compiler or not?
The error I get (with gcc 4.8) is probably the same as what you get with gcc:
|
Yes, we are using 4.8 gcc, same errors here. To me this looks like a On Mon, Jun 23, 2014 at 10:58 AM, Toru Shiozaki [email protected]
web: valeyev.net |
Whenever I used constexpr I used it with static functions. I tried a bit to convert rank() to static but have not been successful yet. |
I need to read the standard, unfortunately, to understand which compiler is On Mon, Jun 23, 2014 at 11:16 AM, Toru Shiozaki [email protected]
web: valeyev.net |
Re: contract. In our discussion the other day, my understanding is that TensorBase refers to a general slice (previously a TensorView, and not necessarily contiguous) and Tensor is derived from TensorBase and IS contiguous. So, if you are writing contract for Tensor then it should be able to assume contiguous memory. Contract for TensorBase in principle can only use the iterator interface. I think contiguous is important enough that it should in addition be a property of Tensor/TensorBase or their corresponding ranges. This would allow contract for TensorBase to have an if statement. Also, tensors with compile time rank can be supported by the contract interface (since it is templated on tensorA, tensorB, tensorC types) and should be able to conform to the tensor concept, so if you want to use fix-rank Tensors, you can write the specialized contract for them. Naoki's original library was for tensors with compile time rank. |
For TensorBase, My idea was that "iterator" is templated in TensorBase class, and Tensor is Maybe this is not the right place... On Tue, Jun 24, 2014 at 5:10 PM, gkc1000 [email protected] wrote:
|
Note that slices can also be contiguous ... I'm not sure if Toru or Miles I think to be able to properly detect the contiguity at compile-time we'd On Tue, Jun 24, 2014 at 7:02 AM, Naoki Nakatani [email protected]
web: valeyev.net |
I do have contiguous slices quite often. E.g., the occupied MO coefficient matrices sliced from the entire MO (and I use column major). |
Yes, slices can be contiguous. This is why I suggest that slices (i.e. TensorBases?) should have a runtime function/property that detects contiguousness. This adds overhead during construction but is probably worth it. Then "if" statements can be used to ensure optimal contraction for slices, in the generic contract function that works on TensorBase. However, for Tensors, I think there should be a separate contract function (template specialization for Tensor) since we know that Tensors are always contiguous. Maybe Tensor is a bad name - it should be DenseTensor. |
At the moment most of the use cases for slicing I foresee in ITensor involve using slices/TensorBases as intermediate objects when get assigned to new dense Tensors. For these cases it would be great if the Tensor constructor automatically checks if TensorBase::contig()==true and if so does a straight std::copy of the memory. While on the topic of naming, I'd suggest keeping the name of TensorView something like "TensorView", "TensorRef", "TensorSlice" which are descriptive of its non-owning behavior. The name "TensorBase" describes more about its implementation rather than its behavior or purpose. |
I was wondering if there's no way to detect contiguousness of slice at the For the name, my concern is that TensorBase in principle doesn't have Toru: Is this a right way? I think it should be "const TensorBase& b = I prefer to use "Tensor" rather than "DenseTensor" since it's simple. I On Wed, Jun 25, 2014 at 12:14 AM, Miles [email protected] wrote:
|
Oh, ok I thought TensorBase was completely replacing TensorView - if it's going to be derived from TensorBase then the name sounds fine to me. |
@naokin I think what you wrote is fine with me. |
I think it is very useful for optimization of contract if one could obtain information on rank() and contiguous() at compile time. Otherwise there will be a bunch of if statements in a single function, which may make the code slower (especially if it is used for small tensors). Any ideas?
The text was updated successfully, but these errors were encountered: