-
Notifications
You must be signed in to change notification settings - Fork 9
Re-implement optimizer Treewidth #95
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
Conversation
|
The failing test is just a hard-coded contraction tree! |
|
Hey, glad to see new progress!
We favor the binary format for multiple reasons,
I think it is guaranted. A contraction complexity is determined by the number (and size) of variables involved in the current step. Binary contraction involves strictly less or equal number of variables when compared with more than two tensors being involved. But I admit this argument is only from the complexity perspective. What about the constant factor? apparently not optimal. e.g. the The best API to produce a binary tree is: OMEinsumContractionOrders.jl/src/Core.jl Line 320 in 6a235ee
You can see it is very cheap method to call. However, it requires the input tree to only have unary and binary nodes. For nary node, we still need a cheap method to determine a contraction order. From this perspective, I think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR. I agree on the key changes in this PR. Before merging, I hope the tests can be fixed. A comment on styling: I personally favor shorter functions, they are easier to test and review.
|
Some benchmarks to justify the change. The tensor network mc_2023_151.json has 18764 tensors and 6221 indices. before julia> optimizer = HyperND(; dis=METISND(), width=50, imbalances=100:10:800);
julia> @time optimize_code(code, size_dict, optimizer);
46.792407 seconds (583.91 M allocations: 34.268 GiB, 13.56% gc time)after julia> optimizer = HyperND(; dis=METISND(), width=50, imbalances=100:10:800);
julia> @time optimize_hyper_nd(optimizer, code, size_dict; binary=false);
13.996466 seconds (26.68 M allocations: 9.982 GiB, 15.67% gc time)
julia> @time optimize_code(code, size_dict, optimizer);
14.616734 seconds (33.42 M allocations: 11.690 GiB, 14.03% gc time, 0.00% compilation time)For sufficiently large tensor networks, binarization triggers a |
|
LGTM. Since no exported APIs change, I tagged a patch version here: JuliaRegistries/General#139722 |
|
@GiggleLiu Hi, I forgot to mention -- I changed the default settings of |
No problem, thank you for the reminder. |
Hi @GiggleLiu I hope you are well.
I just released an update to CliqueTrees.jl. You should notice that the
MFandNDelimination algorithms are much, much faster, now. In fact, they are so fast that a great deal of the running time comes from setting up the problem in treewidth.jl file. The PR rewrites the file, speeding things up. It also fixes this printing bug.I made a couple decisions which I would like your advice on. At this point in the
optimize_treewidthfunction, the contraction schedulecodeis n-ary. As you can see, we "binarize" the tree by callingcode = _optimize_code(code, size_dict, GreedyMethod()). This behavior is triggered by a keyword argumentbinary, which defaults totrue.Do contraction trees have to be binary? If so, is this the "right" way to binarize one? In practice, it seems to preserve the time and space complexity, but I suspect this is not a guarantee. Also, if n-ary trees are acceptable, then we might want to expose the
binarykeyword from the functionoptimize_treewidth.Sincerely,
Richard Samuelson