split_node drops more singular values than it should when using max_truncation_err about tensornetwork HOT 9 CLOSED

Oof. That looks like a false assumption about singular value ordering?

from tensornetwork.

I think it's likely an off by one in the cumsum slicing. I'll investigate it tonight.

from tensornetwork.

Ah, I misread. Actually, I think this is working as intended! What you are calling truncation_error is a vector of the discarded singular values, so the truncation error is actually tf.norm(truncation_error), which is 2.5 in this case.

from tensornetwork.

I'm a bit confused by the meaning of max_truncation_err. I would expect that a value of e.g.
max_truncation_err=0.4 would mean that you discard the smallest n singular values of s such that (fors ordered from large to small)sum(s[n:])<=max_truncation_err. the following code though does something different:

diag = np.sort(np.arange(10).astype(np.float64))
diag /= np.linalg.norm(diag)
print(diag[::-1])
diagonal_array = np.diag(diag)
net = tensornetwork.TensorNetwork()
a = net.add_node(diagonal_array)
u, vh, truncation_error = net.split_node(
    a, left_edges=[a[0]], right_edges=[a[1]], max_truncation_err=0.4)
print(np.sum(truncation_error.numpy()))
print(truncation_error.numpy())

which outputs

[0.53311399 0.4738791  0.41464421 0.35540933 0.29617444 0.23693955
 0.17770466 0.11846978 0.05923489 0.        ]
0.5923488777590923
[0.23693955 0.17770466 0.11846978 0.05923489 0.        ]

what is the rationale behind this?

from tensornetwork.

Yeah I agree with Martin. It would make more sense if sum(trun_err) <= max_truncation_err, the norm doesn't feel natural.

from tensornetwork.

OK I see. It actually depends on the norm you would like to use to compare the truncated and the original matrix. The current implementation uses L2, which is the one we usually use in our tensor network implementations. We could add a flag that allows the user to choose the norm. In any case, we should the convention to the documentation of split_node, because this might confuse people.

from tensornetwork.

@mganahl Yes. It's mentioned that "the norm" is used in the documentation for the svd decomposition, but it should definitely be made clearer.

It would be cool to have the norm configurable! The infinity norm would also be an intuitive choice (truncate so that np.max(discarded_singular_values) <= max_truncation_err). We could use a cumnorm(x, ord=...) op.

from tensornetwork.

Sounds good! The documentation of split_node() only says max_truncation_err: The maximum allowed truncation error, but we should say that we are using the L2 norm (or equivalently Hilbert-Schmidt norm for matrices), i.e. something like
max_truncation_err: The maximum allowed truncation error as measured by the Hilbert-Schmidt norm. I can add it to my latest PR if that's not too confusing.

from tensornetwork.

I agree let's improve the documentation. Though I'd rather you do it in a separate PR

from tensornetwork.