Skip to content
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

[hilbert_space] Simplify algorithm of space_partition::merge_subspaces() #961

Open
wants to merge 1 commit into
base: unstable
Choose a base branch
from

Conversation

krivenko
Copy link
Contributor

The current implementation of space_partition::merge_subspaces() uses recursion, std::function and repeatedly deletes elements from a std::multimap, which results in a significat runtime overhead.

More concerningly, I experienced a stack oveflow caused by the recursion while trying to run the autopartition algorithm for a bigger system.

The new algorithm simply iterates over subspace connections $i\mapsto f$ generated by a single operator $c^\dagger/c$, and merges all subspaces $f$ corresponding to the same $i$.

@Wentzell Wentzell force-pushed the DEV_merge_subspaces_fix branch from e737e9d to 3333c91 Compare March 5, 2025 16:49
@Wentzell Wentzell changed the base branch from 3.3.x to unstable March 5, 2025 16:49
…es()`

The new non-recursive algorithm is simpler, faster and safe from stack
overflows.
@Wentzell
Copy link
Member

Dear @krivenko
Thank you for proposing this simplification.

It is not immediately obvious to me why the new implementation will always lead to the same result as before.

In the previous case you were using the zigzag traversal starting with the upwards connection i0->f0.
At each step of the zig zag you were linking all final subspaces with the same initial subspace to the original i0 or f0 respectively. This would also link subspaces further down the zig zag line to the initial i0 or f0.

This no longer applies in the new implementation, where all you are looking at is which final subspaces can be directly reached from the same initial one.

Can you please clarify? Happy to talk over zoom if thats easier.

@Wentzell Wentzell force-pushed the DEV_merge_subspaces_fix branch from 3333c91 to 884fe1d Compare March 17, 2025 22:12
@krivenko
Copy link
Contributor Author

In the previous case you were using the zigzag traversal starting with the upwards connection i0->f0.
At each step of the zig zag you were linking all final subspaces with the same initial subspace to the original i0 or f0 respectively. This would also link subspaces further down the zig zag line to the initial i0 or f0.

This no longer applies in the new implementation, where all you are looking at is which final subspaces can be directly reached from the same initial one.

The ultimate goal of the algorithm is to make sure that each $c_i$ and $c^\dagger_i$ generates only one-to-one connections between subspaces. This condition can be checked and enforced independently for each operator -- there is, in fact, no real need to work with pairs $(c_i, c^\dagger_i)$ and to do any sort of back-and-forth iteration. Once the condition is fulfilled for an operator, it cannot be violated by any further subspace mergers.

Therefore, one can simply take an operator, merge all final subspaces corresponding to the same initial subspace, and proceed to the next operator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants