ITensorNetwork type fixes and other minor refactors. - #175
Conversation
|
Your PR no longer requires formatting changes. Thank you for your contribution! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #175 +/- ##
==========================================
+ Coverage 86.09% 87.19% +1.09%
==========================================
Files 15 16 +1
Lines 669 687 +18
==========================================
+ Hits 576 599 +23
+ Misses 93 88 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
dee5559 to
faa56ff
Compare
| return tn | ||
| end | ||
|
|
||
| function supportof(tn::AbstractGraph, op::ITensorOperator) |
There was a problem hiding this comment.
I think maybe I'd call this operator_support, just to not make it sound so generic, i.e. we don't know what the "scope" of this function will be. Also, what about constraining to tn::AbstractITensorNetwork?
There was a problem hiding this comment.
I worry about constraining such functions as then they no longer "just work" on e.g. wrappers that do not subtype AbstractITensorNetwork.
As for the name, I have no strong opinions. I have been drafting the ITensorNetworkOperator object, where this function would also have a method for. I don't know if that changes your opinion (although operator_support would still work in that case).
There was a problem hiding this comment.
"wrappers that do not subtype AbstractITensorNetwork": We could always generalize when we have those cases (or maybe you have one already?). "I don't know if that changes your opinion": I think that still fits, i.e. I think operator_support would assume some structure of the operator, like it has input and output indices and you are specifically matching against input indices. We could always generalize the name as the use cases generalize.
There was a problem hiding this comment.
There is no specific case. It is more of a case of me deciding to choose the default to be AbstractGraph when it does not introduce type piracy or ambiguity after having to change this multiple times to accommodate QuotientView.
Happy to change the name.
| # PERF: fast lookup compared to `AbstractITensorNetwork` fallback. | ||
| dimnamevertices(tn::ITensorNetwork, name) = tn.dimname_vertices[name] | ||
| function dimnamevertices(tn::ITensorNetwork, name) | ||
| return get(tn.dimname_vertices, name, Set{vertextype(tn)}()) |
There was a problem hiding this comment.
I think this makes more sense, but was this inspired by a particular use case?
There was a problem hiding this comment.
It is for consistency with the fallback method (which returns an empty set).
7225dab to
48534f9
Compare
…alse instead of erroring This is inline with the `Graphs` behaviour.
… not in dictionary This is now consistant with the fallback defn of `dimnamevertices`. would error previously.
… on a tensor network.
Avoids some minor code duplication.
Fix imports in `apply_operators.jl`
…patch Convention from `MatrixAlgebraKit`.
…akes edges directly.
…nergy` as its negative. The function returns the BP estimate of log Z, which is the free entropy; the free energy is -log Z. Adds tests for the relation between the two and for a vanishing edge overlap. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…t arg. This is for consistency with `vertex_scalars`.
12d850e to
e1702bd
Compare
e1702bd to
8a9b645
Compare
|
Looks good to me, thanks! Can you bump the version? |
3 participants
Fixes and cleanups to the ITensorNetwork type, the BP message cache, and gate application.
add_edge!/rem_edge!on ITensorNetwork return false instead of throwing.dimnameverticesreturns an empty set for a name the network does not hold.operator_support(tn, op): the vertices carrying the operator's input index names. An absent name contributes no vertex; a name on several vertices throws.dimname_verticesand the underlying graph half-updated.finalize_substate!takes both subsolve and solve objects as arguments.bethe_free_energy's duplicated log-sum factored intosumlog.