Accumulate ParallelLinearQubitOperator matvecs in-place (#1410) - #1450
Open
rosspeili wants to merge 1 commit into
Open
Accumulate ParallelLinearQubitOperator matvecs in-place (#1410)#1450rosspeili wants to merge 1 commit into
rosspeili wants to merge 1 commit into
Conversation
…antumlib#1410) Accumulate partial matvecs into one buffer with in-place += instead of functools.reduce(numpy.add), for both the single-process and multiprocess paths. Defer forkserver setup until processes > 1 so the single-process path works on Windows.
Contributor
There was a problem hiding this comment.
Code Review
This pull request optimizes memory usage and improves platform compatibility for ParallelLinearQubitOperator. It replaces functools.reduce(numpy.add, ...) with a new helper function _accumulate_vectors that performs in-place vector accumulation to avoid intermediate allocations. Additionally, it ensures that the 'forkserver' multiprocessing start method is only set when spawning multiple processes, allowing the single-process path to run on platforms like Windows. Unit tests have been added to cover the new helper function and the single-process execution path. There are no review comments, so I have no feedback to provide.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the suggestion from #1410: avoid
functools.reduce(numpy.add, …)when combining partial matvecs, which was allocating a new full-sized array for every partial sum.Used the approach from GCA and went a bit further:
+=accumulation for both the single-process and multiprocess paths (same cost when combining worker results).forkserverwhenprocesses > 1, so the single-process path works on Windows.There is one nuance with
processes=1, where operator grouping usually yields a single group so the biggest + is on the multiprocess reduce. Happy to trim or adjust if its better to keep the change scoped only to the single-process path.Tests passed, and will wait for CI to see if I missed anything.
Fixes #1410