Closed ryanmrichard closed 2 years ago
The commutator also doesn't work as a one-liner:
comm() = F(mu,nu)*S(nu, lambda)*rho(lambda, sigma) - rho(mu, nu)*S(nu, lambda)*F(lambda, sigma);
error:
error: no match for ‘operator-’ (operand types are ‘std::tuple<tamm::LoopSpec, tamm::LabeledTensor
, tamm::LabeledTensor , tamm::LabeledTensor >’ and ‘std::tuple<tamm::LoopSpec, tamm::LabeledTensor , tamm::LabeledTensor , tamm::LabeledTensor >’)
Edit: finally got it:
```.cpp
comm1(mu, lambda) = F(mu,nu)*S(nu, lambda);
comm(mu, nu) = comm1(mu, lambda)*rho(lambda, nu);
comm1(mu, lambda) = rho(mu, nu)*S(nu, lambda);
comm(mu, nu) += comm1(mu, lambda)*F(lambda, nu);
```
@ryanmrichard I think TAMM is only for single-op expressions only (binary or unary) ... not for arbitrary trees of ops. @sriramkmoorthy please correct if this is not the case.
@evaleev looking at the current tests I think you might be right. I swear the CCSD test (which I can't seem to find) did more on one line. Writing something like the Fock matrix on multiple lines is painful, maybe this issue should be merged with #26?
@evaleev Yes, Arbitrary expressions are not supported.
@ryanmrichard In TAMM, an operation can only be of the form c [+]= [alpha] * a [ * b]
We could support sum of such products (not there yet), for ex: c += al1*a1*b1 + al2*a2*b2 + ...
which we could internally split into single-op expressions.
@ajaypanyala sounds good although I'd also like to request support for subtraction (even if it's just a wrapper around negated addition).
That would restrict a lot of possible optimizations with respect to data movement. For example, Ryan's:
comm() = F(mu,nu)S(nu, lambda)rho(lambda, mu) - rho(mu, nu)S(nu, lambda)F(lambda, mu);
By breaking this up in two, does it now have to get the same set of matrix elements twice? As this equation is on the full matrix, it feels like there might be too much data movement because of the operator choice?!
On Wed, May 30, 2018 at 11:05 AM, Ajay Panyala notifications@github.com wrote:
@evaleev https://github.com/evaleev Yes, Arbitrary expressions are not supported. @ryanmrichard https://github.com/ryanmrichard In TAMM, an operation can only be of the form c [+]= [alpha] a [ b] We could support sum of such products (not there yet), for ex: c += al1a1b1 + al2a2b2 + ... which we could internally split into single-op expressions.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-393197911, or mute the thread https://github.com/notifications/unsubscribe-auth/AGa9cui3Fk5642ihJLS0hcla9tFa3GI7ks5t3rVEgaJpZM4UTY08 .
@wadejong under the hood I think it sees the commutator as the full expression because nothing is evaluated until we call execute
on the DAG. So I think the breaking up of the expression is just a pain point for the writer.
@ryanmrichard I would hope so. Would be good to understand from Sriram and co how they will eventually construct the execution graph.
On Wed, May 30, 2018 at 3:32 PM, Ryan Richard notifications@github.com wrote:
@wadejong https://github.com/wadejong under the hood I think it sees the commutator as the full expression because nothing is evaluated until we call execute on the DAG. So I think the breaking up of the expression is just a pain point for the writer.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-393288988, or mute the thread https://github.com/notifications/unsubscribe-auth/AGa9cjxZpAifc6wwqgerNNjq9uJS4T6Vks5t3vPegaJpZM4UTY08 .
@wadejong We do not plan to support more complex expressions than the syntax @ajaypanyala presented earlier. Doing the optimization you mention involve several steps: binarization (factoring into binary terms), common sub-expression identification, followed by fusion. We plan to do these, but not anytime soon. So I would program assume they are not available. We will add some fusion API to support data reuse across operations, so all of this can be done manually by the user. We plan to do this after we get the iterators into a PR.
@ryanmrichard Subtraction support is now added in ops-parser
branch, Expressions of type
F(mu, nu) -= K(mu,nu);
should work now.
Based on the errors I'm getting for the SCF code I presume we still can't add two tensors together in one line? I'm trying:
sch(H("mu", "nu") = T("mu", "nu") + V("mu", "nu")).execute()
I can work around it, by first assigning T
to H
and then accumulating into H
, but that seems unnecessarily complicated.
Correct. We can add this support (for addition only) at a later point.
On Thu, Sep 6, 2018 at 7:06 AM Ryan Richard notifications@github.com wrote:
Based on the errors I'm getting for the SCF code I presume we still can't add two tensors together in one line? I'm trying:
sch(H("mu", "nu") = T("mu", "nu") + V("mu", "nu")).execute()
I can work around it, by first assigning T to H and then accumulating into H, but that seems unnecessarily complicated.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-419106066, or mute the thread https://github.com/notifications/unsubscribe-auth/ACag6JzuYUFW1HMnazHnkOC76iVzkH6vks5uYSv1gaJpZM4UTY08 .
Why addition only? Seems like subtraction would be needed too.
Sorry, I meant addition/subtraction. Specifically, we will not support non-binary multiplications, until we have good logic for factoring them. On Thu, Sep 6, 2018 at 7:39 AM Ryan Richard notifications@github.com wrote:
Why addition only? Seems like subtraction would be needed too.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
Hi Sriram,
A dumb question from ignorant me: What is a non-binary multiplication? Multiplication by a scalar is clearly a trivial (unary) operation, multiplying 2 tensors is a binary operation that we definitely need, are there any other multiplication operations? Do you mean something like A*=B perhaps, where A and B are tensors?
Best wishes,
Huub
Hubertus van Dam, 631-344-6020, hvandam@bnl.gov Brookhaven National Laboratory
From: sriramkmoorthy notifications@github.com Reply-To: NWChemEx-Project/TAMM reply@reply.github.com Date: Thursday, September 6, 2018 at 12:25 PM To: NWChemEx-Project/TAMM TAMM@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [NWChemEx-Project/TAMM] Expression Type Error (#27)
Sorry, I meant addition/subtraction. Specifically, we will not support non-binary multiplications, until we have good logic for factoring them. On Thu, Sep 6, 2018 at 7:39 AM Ryan Richard notifications@github.com wrote:
Why addition only? Seems like subtraction would be needed too.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-419156684, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AEcvnrwp0ohMqdwvgd761DzCWCtNoV4Nks5uYUw0gaJpZM4UTY08.
@hjjvandam I believe he means something like T("i", "l") = A("i", "j")*B("j", "k")*C("k", "l");
. In such a case you have to figure out whether to do A*B
or B*C
first and they don't have logic for that yet.
I meant multiplication chains as in:
C(i,l) += A(i,j)B(j,k)C(k,l) //index labels used for illustration
Here TAMM needs to decide how to factorize (or parenthesize if you will) ABC into (AB)C or A(BC).
On Thu, Sep 6, 2018 at 9:56 AM Hubertus van Dam notifications@github.com wrote:
Hi Sriram,
A dumb question from ignorant me: What is a non-binary multiplication? Multiplication by a scalar is clearly a trivial (unary) operation, multiplying 2 tensors is a binary operation that we definitely need, are there any other multiplication operations? Do you mean something like A*=B perhaps, where A and B are tensors?
Best wishes,
Huub
Hubertus van Dam, 631-344-6020, hvandam@bnl.gov Brookhaven National Laboratory
From: sriramkmoorthy notifications@github.com Reply-To: NWChemEx-Project/TAMM reply@reply.github.com Date: Thursday, September 6, 2018 at 12:25 PM To: NWChemEx-Project/TAMM TAMM@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [NWChemEx-Project/TAMM] Expression Type Error (#27)
Sorry, I meant addition/subtraction. Specifically, we will not support non-binary multiplications, until we have good logic for factoring them. On Thu, Sep 6, 2018 at 7:39 AM Ryan Richard notifications@github.com wrote:
Why addition only? Seems like subtraction would be needed too.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub< https://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-419156684>, or mute the thread< https://github.com/notifications/unsubscribe-auth/AEcvnrwp0ohMqdwvgd761DzCWCtNoV4Nks5uYUw0gaJpZM4UTY08>.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-419166936, or mute the thread https://github.com/notifications/unsubscribe-auth/ACag6Hb1XbwDHJELwi0eyYW1Mu1Ol12gks5uYVPSgaJpZM4UTY08 .
Ok, that is a potentially hairy question. Is it in principle possible that if you solve this ternary problem that you can use the solution strategy recursively to solve quartenary and higher order problems as well, i.e. Z+=ABCDetc.?
Hubertus van Dam, 631-344-6020, hvandam@bnl.gov Brookhaven National Laboratory
From: sriramkmoorthy notifications@github.com Reply-To: NWChemEx-Project/TAMM reply@reply.github.com Date: Thursday, September 6, 2018 at 1:00 PM To: NWChemEx-Project/TAMM TAMM@noreply.github.com Cc: "Van Dam, Hubertus" hvandam@bnl.gov, Mention mention@noreply.github.com Subject: Re: [NWChemEx-Project/TAMM] Expression Type Error (#27)
I meant multiplication chains as in:
C(i,l) += A(i,j)B(j,k)C(k,l) //index labels used for illustration
Here TAMM needs to decide how to factorize (or parenthesize if you will) ABC into (AB)C or A(BC).
On Thu, Sep 6, 2018 at 9:56 AM Hubertus van Dam notifications@github.com wrote:
Hi Sriram,
A dumb question from ignorant me: What is a non-binary multiplication? Multiplication by a scalar is clearly a trivial (unary) operation, multiplying 2 tensors is a binary operation that we definitely need, are there any other multiplication operations? Do you mean something like A*=B perhaps, where A and B are tensors?
Best wishes,
Huub
Hubertus van Dam, 631-344-6020, hvandam@bnl.gov Brookhaven National Laboratory
From: sriramkmoorthy notifications@github.com Reply-To: NWChemEx-Project/TAMM reply@reply.github.com Date: Thursday, September 6, 2018 at 12:25 PM To: NWChemEx-Project/TAMM TAMM@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [NWChemEx-Project/TAMM] Expression Type Error (#27)
Sorry, I meant addition/subtraction. Specifically, we will not support non-binary multiplications, until we have good logic for factoring them. On Thu, Sep 6, 2018 at 7:39 AM Ryan Richard notifications@github.com wrote:
Why addition only? Seems like subtraction would be needed too.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub< https://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-419156684>, or mute the thread< https://github.com/notifications/unsubscribe-auth/AEcvnrwp0ohMqdwvgd761DzCWCtNoV4Nks5uYUw0gaJpZM4UTY08>.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-419166936, or mute the thread https://github.com/notifications/unsubscribe-auth/ACag6Hb1XbwDHJELwi0eyYW1Mu1Ol12gks5uYVPSgaJpZM4UTY08 .
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-419168027, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AEcvngPvqYa85ls_YQ08sNhoKmlPiNzHks5uYVSigaJpZM4UTY08.
The key question is one of intermediates (and their collective allocation).
In principle, yes. Basically, the scheduler would infer the intermediate types, and create/destroy those tensors on the fly.
Unless this is critical, I want to do this only after I have memory usage tracking in place throughout TAMM. That way, I can factorize accounting for memory bound.
On Thu, Sep 6, 2018 at 10:49 AM Hubertus van Dam notifications@github.com wrote:
Ok, that is a potentially hairy question. Is it in principle possible that if you solve this ternary problem that you can use the solution strategy recursively to solve quartenary and higher order problems as well, i.e. Z+=ABCDetc.?
Hubertus van Dam, 631-344-6020, hvandam@bnl.gov Brookhaven National Laboratory
From: sriramkmoorthy notifications@github.com Reply-To: NWChemEx-Project/TAMM reply@reply.github.com Date: Thursday, September 6, 2018 at 1:00 PM To: NWChemEx-Project/TAMM TAMM@noreply.github.com Cc: "Van Dam, Hubertus" hvandam@bnl.gov, Mention < mention@noreply.github.com> Subject: Re: [NWChemEx-Project/TAMM] Expression Type Error (#27)
I meant multiplication chains as in:
C(i,l) += A(i,j)B(j,k)C(k,l) //index labels used for illustration
Here TAMM needs to decide how to factorize (or parenthesize if you will) ABC into (AB)C or A(BC).
On Thu, Sep 6, 2018 at 9:56 AM Hubertus van Dam notifications@github.com
wrote:
Hi Sriram,
A dumb question from ignorant me: What is a non-binary multiplication? Multiplication by a scalar is clearly a trivial (unary) operation, multiplying 2 tensors is a binary operation that we definitely need, are there any other multiplication operations? Do you mean something like A*=B perhaps, where A and B are tensors?
Best wishes,
Huub
Hubertus van Dam, 631-344-6020, hvandam@bnl.gov Brookhaven National Laboratory
From: sriramkmoorthy notifications@github.com Reply-To: NWChemEx-Project/TAMM reply@reply.github.com Date: Thursday, September 6, 2018 at 12:25 PM To: NWChemEx-Project/TAMM TAMM@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [NWChemEx-Project/TAMM] Expression Type Error (#27)
Sorry, I meant addition/subtraction. Specifically, we will not support non-binary multiplications, until we have good logic for factoring them. On Thu, Sep 6, 2018 at 7:39 AM Ryan Richard notifications@github.com wrote:
Why addition only? Seems like subtraction would be needed too.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<
https://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-419156684>,
or mute the thread<
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-419166936>,
or mute the thread < https://github.com/notifications/unsubscribe-auth/ACag6Hb1XbwDHJELwi0eyYW1Mu1Ol12gks5uYVPSgaJpZM4UTY08>
.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub< https://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-419168027>, or mute the thread< https://github.com/notifications/unsubscribe-auth/AEcvngPvqYa85ls_YQ08sNhoKmlPiNzHks5uYVSigaJpZM4UTY08>.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-419183273, or mute the thread https://github.com/notifications/unsubscribe-auth/ACag6Le1QpHKD7MitQPfaSxWFWiwKE78ks5uYWBDgaJpZM4UTY08 .
This is a relatively solved problem (n log n
complexity is solved by electronic structure standards). See: :link: https://en.wikipedia.org/wiki/Matrix_chain_multiplication.
With tensors, contractions commute and the problem is NP-complete. There are many op-min efforts out there. Memory-bounded op-min requires more work.
On Thu, Sep 6, 2018 at 10:58 AM Ryan Richard notifications@github.com wrote:
This is a relatively solved problem (n log n complexity is solved by electronic structure standards). See: https://en.wikipedia.org/wiki/Matrix_chain_multiplication.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
I suppose that sparsity makes the matter even trickier as it becomes harder to say what the cost of even a simple matrix-matrix multiplication is when both matrices are sparse.
Hubertus van Dam, 631-344-6020, hvandam@bnl.gov Brookhaven National Laboratory
From: sriramkmoorthy notifications@github.com Reply-To: NWChemEx-Project/TAMM reply@reply.github.com Date: Thursday, September 6, 2018 at 2:05 PM To: NWChemEx-Project/TAMM TAMM@noreply.github.com Cc: "Van Dam, Hubertus" hvandam@bnl.gov, Mention mention@noreply.github.com Subject: Re: [NWChemEx-Project/TAMM] Expression Type Error (#27)
With tensors, contractions commute and the problem is NP-complete. There are many op-min efforts out there. Memory-bounded op-min requires more work.
On Thu, Sep 6, 2018 at 10:58 AM Ryan Richard notifications@github.com wrote:
This is a relatively solved problem (n log n complexity is solved by electronic structure standards). See: https://en.wikipedia.org/wiki/Matrix_chain_multiplication.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-419188503, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AEcvnptS45wghOtqFFKDACOPLiNCf_lBks5uYWPFgaJpZM4UTY08.
Yes, I think sparsity is the truly open area.
On Thu, Sep 6, 2018 at 11:08 AM Hubertus van Dam notifications@github.com wrote:
I suppose that sparsity makes the matter even trickier as it becomes harder to say what the cost of even a simple matrix-matrix multiplication is when both matrices are sparse.
Hubertus van Dam, 631-344-6020, hvandam@bnl.gov Brookhaven National Laboratory
From: sriramkmoorthy notifications@github.com Reply-To: NWChemEx-Project/TAMM reply@reply.github.com Date: Thursday, September 6, 2018 at 2:05 PM To: NWChemEx-Project/TAMM TAMM@noreply.github.com Cc: "Van Dam, Hubertus" hvandam@bnl.gov, Mention < mention@noreply.github.com> Subject: Re: [NWChemEx-Project/TAMM] Expression Type Error (#27)
With tensors, contractions commute and the problem is NP-complete. There are many op-min efforts out there. Memory-bounded op-min requires more work.
On Thu, Sep 6, 2018 at 10:58 AM Ryan Richard notifications@github.com wrote:
This is a relatively solved problem (n log n complexity is solved by electronic structure standards). See: https://en.wikipedia.org/wiki/Matrix_chain_multiplication.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub< https://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-419188503>, or mute the thread< https://github.com/notifications/unsubscribe-auth/AEcvnptS45wghOtqFFKDACOPLiNCf_lBks5uYWPFgaJpZM4UTY08>.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-419189527, or mute the thread https://github.com/notifications/unsubscribe-auth/ACag6CZP6Kk1DI9HkSgMu7Nm4WRgZAUjks5uYWSQgaJpZM4UTY08 .
For anyone else interested I found a paper authored by @robertjharrison (and others) that hits on some of the points just mentioned; a link is here.
Errr...I thought I understood how to do a contraction, but guess not. The following line
sch.allocate(rho)(rho(mu, nu) = C(mu, p) * C(nu, p)).execute();
is giving me the error:
ERROR: file:tamm/ops.hppfunction:MultOp line:1180. This is not implemented EXPECTS failed. Condition: tamm/ops.hpp 1201 !isassign
as best as I can tell this is the same syntax I had used above and it worked...
This is a different error and a short-term limitation in TAMM.
We only have a general multiplication implemented (or rather current enabled).With this, one can only do:
C+= A*B
and not
C= A*B
For common cases, we can quickly do a check and enable the assignment syntax.
On Tue, Sep 18, 2018 at 2:21 PM Ryan Richard notifications@github.com wrote:
Errr...I thought I understood how to do a contraction, but guess not. The following line
sch.allocate(rho)(rho(mu, nu) = C(mu, p) * C(nu, p)).execute();
is giving me the error:
ERROR: file:tamm/ops.hppfunction:MultOp line:1180. This is not implemented EXPECTS failed. Condition: tamm/ops.hpp 1201 !isassign
as best as I can tell this is the same syntax I had used above and it worked...
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NWChemEx-Project/TAMM/issues/27#issuecomment-422559937, or mute the thread https://github.com/notifications/unsubscribe-auth/ACag6PfE-BvcR_p2FD5fZiCbHwF-Pen-ks5ucWPFgaJpZM4UTY08 .
I'm apparently still missing something.
sch.allocate(rho)
(rho(mu, nu) = 0)
(rho(mu, nu) += C(mu, p) * C(nu, p)).execute();
fails with:
tamm/kernels/assign.hpp:446: void tamm::kernels::assign(T, const SizeVec&, const IntLabelVec&, T, const T, const SizeVec&, const IntLabelVec&, bool) [with T = double; tamm::SizeVec = std::vector<tamm::StrongNum<tamm::OffsetSpace, long unsigned int> >; tamm::IntLabelVec = std::vector
]: Assertion `ddims.size() == sdims.size()' failed.
@ryanmrichard Can you give some more details about the Tensors and tiling?
@erdalmutlu each of the modes of the various tensors are spanned by the same tiled index space (albeit copies in case this could be related to #45) which is 7 basis functions long and tiled by atom (the first basis function goes with the first atom, the next 5 with the second atom, and the last one with the last atom).
@ryanmrichard It took me awhile but I was able to find the reason for this issue. I think it is also related to the question #52.
So the problem is both const auto AOs = C.tiled_index_spaces()[0];
and const auto MOs = C.tiled_index_spaces()[1];
are the same TiledIndexSpace
(as C is AO by AO after eigenvectors()
call) so when you construct labels using these spaces you are basically using the same space. And as the calls to labels<N>(...)
uses labels (integers) starting from 0 up to N-1 to construct the TiledIndexLabel
s, mu
and p
end up using the same index space and label (0) which causes it to be dropped while constructing the loop nests.
So if you give label value like auto [p] = MOs.labels<1>("all", 2);
or as TiledIndexSpace
s are the same construct labels like auto [mu, nu, p] = AOs.labels<3>("all");
the problem will be solved.
@erdalmutlu I don't really understand what the problem was, but like you said auto [mu, nu, p] = AOs.labels<3>("all")
fixes the problem for me, so thanks.
The expression for the restricted Fock matrix:
generates the following error:
Doing it in two steps:
generates the following error:
and breaking it into three steps:
generates the following error:
Only when I do:
does it finally compile.