google / TensorNetwork

A library for easy and efficient manipulation of tensor networks.
Apache License 2.0
1.79k stars 352 forks source link

Giving ncon a pythonic makeover #863

Open repst opened 3 years ago

repst commented 3 years ago

The function ncon is powerful and beloved by veteran tensor networkers. Yet despite its prowess, its matlabian exterior prevents it from feeling like a natural python function. First, edge indices that start at 1 don't mesh well with python nor with TN in general where tensor axes that start at index 0. Second, python has a nice alternative to passing in two lists of associated things.

Here are a few ideas on how to adapt ncon to python.

Given tensors left, mid and right, an example of the current syntax is: ncon([left, mid, right], [[1, -1], [1, -2, 2], [2, -3]])

If instead of negative numbers, complex numbers are used to represent dangling edges, 0-based indexing is possible. In addition, replacing the two related lists with a dictionary brings edge indices next to their associated tensors, removes a nesting of lists and reduces the number of function arguments by one: ncon({left:[0, 0j], mid:[0, 1j, 1], right: [1, 2j]})

Alternatively, one could use strings instead of lists with d representing dangling edges: ncon({left:'0, 0d', mid:'0, 1d, 1', right:'1, 2d'})

I'd be curious to hear thoughts from the various stakeholders, especially the black belt ncon users.

mganahl commented 3 years ago

Hey @repst thanks for the proposal. Sounds very interesting to me! Let me think a bit about this. Also roping in @gevenbly and @alewis on this one!

chaserileyroberts commented 3 years ago

This seams very much in the spirit of the "functional node" design we discussed months prior.

The idea was very similar to what you are describing, only instead of wrapping everything in a single ncon function, you would build the contraction like

left = tn.Tensor(...)
mid = tn.Tensor(...)
right = tn.Tensor(...)
resultbuilder = left['0,0d'] @ mid['0, 1d, 1'] @ right['1, 2d']
result = resultbuilder['0d,1d,2d'].to_tensor() # tn.Tensor
alewis commented 3 years ago

oh no

alewis commented 3 years ago

wait didn't Martin and Glen implement the string thing as "fancy ncon"? what happened to that?

anyway so basically the concern is how to handle contractions involving, say, 10 or more tensors with 15 legs each or something. in addition one needs a means to specify the contraction order, ideally without having to rewrite the whole thing

it's felt by some that ncon improves this situation compactly and by others that it leads to weird and ugly code with three 900 character lines. I personally maintain a skillful balance of harmonious neutrality equal to that of the resting crane.

alewis commented 3 years ago

anyway ncon probably needs to exist roughly as is because the old tn guard will otherwise flatly refuse to use the library. however alternative interfaces useful to others are welcome, at least to my mind.

chaserileyroberts commented 3 years ago

Ultimately, it seems like it would essentially be a shim to just calling ncon outright with a different user facing API. I welcome PRs for new desired interfaces! No reason we can't evolve things while keeping the old ncon around.

gevenbly commented 3 years ago

I have a couple of issues with the syntax @repst has suggested. (i) It is a relatively common occurrence to contract networks containing many copies of the same tensor. I don't see how this would work using a dictionary, unless each tensor copy was first given a distinct name (which is something that should be avoided). (ii) It is also a relatively common occurrence to evaluate the same network geometry with different sets of input tensor lists, such as:

TA = ncon(tensorsA,connects)
TB = ncon(tensorsB,connects)

I may be missing something, but I don't see how this could be handled in an elegant way when using dicts. It is also sometimes necessary to evaluate different network geometries that contain exactly the same set of tensors. For these reasons, having the two separate arguments to ncon (a list of tensors and a list of connections) is often quite convenient.

In general, I don't have any problem with the thought of introducing a more pythonic version of ncon, but I think the ncon name should be reserved for a function which remains compatible with the existing syntax. I think that it is very useful to be able to easily port ncon based tensor network codes between Python, Julia and Matlab.

mganahl commented 3 years ago

As Glen has pointed out, using a single input tensor many times within ncon would not be possible with a dict without introducing additional steps for the user. I agree that this is something that we want to avoid (calls to ncon should remain as compact as they are now).

As @alewis mentioned, the current version of ncon supports strings for labelling tensor legs. The idea of using imaginary numbers to denote dangling edges sounds interesting. This convention should though be as seamlessly as possible be applicable to string labels. At the moment we are using a hyphen ("-") to denote dangling edges with string labels, i.e.

AB = ncon([A,B],[['-final_row', 'contracted'],['contracted','-final_column']])

which looks similar to the minus sign for integers.

While I'm all in favour of developing ideas for new interfaces, I think we should avoid having too many different APIs available to the user in parallel. This will foster confusion and uncertainty on the user side. At the moment, we already have ncon and the Edge-Node API. Adding a third distinct option here (unless added in a dedicated experimental folder) seems undesirable.

alewis commented 3 years ago

I actually like the interface Chase suggested in this thread, to be clear. In many (not all) cases I think it will lead to more comprehensible code, for example by allowing a complicated tensor network with a recursive structure to be built as a recursion. Mostly I think we dropped the matter because we decided the numerics took a higher priority.

chaserileyroberts commented 3 years ago

While I'm all in favor of developing ideas for new interfaces, I think we should avoid having too many different APIs available to the user in parallel.

I would generally agree, but in this case I think we'll be ok. Like you said we only have 2 APIs at the moment (the all-in-one ncon and the node/edge object oriented design), and each have some pretty strong weaknesses. Ncon becomes unmanageable beyond a handful of tensors, and the node/edge design is really cumbersome when you want to do anything beyond basic contractions.

I think I can build a prototype of the functional design that's built off the existing ncon pretty easily.

repst commented 3 years ago

Thanks for indulging the new guy stirring up trouble. This is very helpful discussion. Let me stuff some of the worms back in the can for the moment and focus on the most important part (to me): mixed use of 1-based and 0-based indexing in the TN library. This is a bit of a sore subject as my coworker has spent many hours tracking down a bug in our code due to iTensor's 1-based indexing in 0-based C++.

@gevenbly, what would you think about a minimal change to ncon syntax for languages with 0-based indexing? Using imaginary instead of negative numbers to indicate dangling edges seems the most similar to me but there are other options as well. Thinking about implications for TensorTrace, since one already specifies which language to use, it could adapt the ncon indexing to the language.

alewis commented 3 years ago

Does base Python support imaginary int types? I think it would be sorta bad to make the labels floats.

repst commented 3 years ago

@mganahl's point about keeping the dangling flag similar for string types is important as well.

gevenbly commented 3 years ago

@repst, I think that ncon could be written to simultaneously accommodate 0-based and 1-based indexing, and it is a good idea to do so. This could be done using the existing syntax, where one could either label the dangling indices [-0,-1,-2,...] or [-1,-2,-3,...]. The only restriction would be, in the former case, that 0 could not be used as a label over the internal (contracted) indices. This should not be a problem as the labels over the internal indices are only dummy labels so it does not matter what we call them (i.e. whether one labels internal indices as [1,2,3,...] or [8,12,15...] or [1,2,'a1','a2',...] does not make any difference, as they are just used to match pairs of contracted indices)

chaserileyroberts commented 3 years ago

Yeah Glen's answer might be the correct one. We currently disallow 0, so reallowing it and always making it a dangling leg might be ok.

chaserileyroberts commented 3 years ago

@alewis @mganahl Didn't 0 have a special meaning in the original matlab ncon?

alewis commented 3 years ago

I like Glen's answer. I don't really know how the original matlab ncon worked, so I'll leave that to @mganahl

repst commented 3 years ago

I'm happy with @gevenbly's solution too as it resolves the main issue. I think it could be made clear in the documentation that -0 should be used instead of 0 by convention for code readability.

repst commented 3 years ago

Oh, @gevenbly, do you mind providing a code snippet of repeated use of the same tensor with ncon? I think it would be useful to have that documented here lest I suggest dictionaries again some day!

chaserileyroberts commented 3 years ago

@repst Here is a short example of applying an H gate twice.

a = np.array(...)
H = np.array(...)
result = tn.ncon([a, H, H], [[1], [1, 2], [2, -1]])
gevenbly commented 3 years ago

The version of ncon on arxiv had something called 'zeros-in-sequence' notation, where the outer product of two tensors could be understood as having two tensors connected by a trivial-dim index labelled 0 by convention. This could be used to force an outer product to be evaluated in the middle of a contraction. This was something introduced solely at the discretion of Rob Pfeifer (who wrote that ncon version). I never used this feature myself, nor do I feel that it is a useful thing to have (as I don't think there exists any cases in which this would result in a more efficient contraction).

chaserileyroberts commented 3 years ago

Great! We should allow 0 as a dangling leg then. Erroring out when seeing multiple 0s should fix any (unlikely) porting issues.

mganahl commented 3 years ago

I'll assign myself unless someone else is eager to fix it!

repst commented 3 years ago

Erroring out when seeing multiple 0s should fix any (unlikely) porting issues.

I assume we'll still want open batch labels.

mganahl commented 3 years ago

That's right, multiple free labels are fine.