Make multiprocessing calls aware of parallelism used during testing

[mhucka]

Pytest on parallel systems was taking far more time to run than it should have. The problem turned out to be due to the fact that some of the OpenFermion code used multiprocessing without taking account of the available CPU resources on the system. When running parallel pytests (with pytest-xdist), this resulted in the host system being vastly overcommitted: system load would skyrocked, and on a 64-core Linux system, it would take 45 minutes to run pytest -n auto -m "not slow".

This PR changes the way that available CPUs are counted to include a check for whether parallely pytest workers are being used. This is done in two ways:

• Change the OpenFermion classes that use multiprocessing directly (e.g., src/openfermion/linalg/linear_qubit_operator.py)

• Use the threadpoolctl package to limit how many threads native libraries such as BLAS use, so that packages like SciPy and NumPy don't end up using threads without control.

Additional changes:

• Add the threadpoolctl package to the set of pytest dependencies in dev_tools/requirements/deps/pytest.txt.

• Move src/openfermion/conftest.py to the top level, for more consistent pytest behavior when it's invoked in different ways.

• Change check/pylint-changed-files and check/pytest-and-incremental-coverage to not use parallism by default, and to pass command line flags to pylint and pytest so that users or calling scripts can control when flags like -n auto are used. This makes them more consistent with other scripts in check/.

• Change check/all to pass flags for parallel execution to the other scripts, and to accept a new option --no-parallel to turn off that behavior. In addition, make check/all accept short-form argument names, shorten --apply-format-changes to --apply-changes, and rewrite some non-Bash constructs to be consistently Bash throughout.

• Update CONTRIBUTING.md to mention the use of -n auto with pytest, and the use of check/all instead of a long list of individual final checks.

Some of this work was done with the help of Gemini CLI.

[gemini-code-assist]

Code Review

This pull request introduces a new utility function get_available_cpu_count to dynamically determine available CPU cores while respecting process affinity, Docker limits, and pytest-xdist workers. It replaces standard CPU count calls across the codebase and adds a pytest fixture to limit thread pools. The review feedback correctly identifies a double-division bug in conftest.py where the CPU count is divided by the worker count twice, and provides the necessary test updates to accompany the fix.

[mhucka]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a new utility function get_available_cpu_count to safely determine the number of available CPU cores while respecting process affinity, Docker limits, and pytest-xdist worker counts. This utility is integrated across the codebase to replace standard multiprocessing.cpu_count() and os.cpu_count() calls, and a new pytest fixture is added to limit thread pools during parallel testing. Feedback on these changes highlights two main issues: first, setting MKL_NUM_THREADS in thc_factorization.py and thc_objectives.py is ineffective because get_available_cpu_count transitively imports numpy and scipy before the environment variable is set; second, using ParallelLinearQubitOperator when only one process is available introduces unnecessary multiprocessing overhead, suggesting a fallback to sequential execution.

[mhucka]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a helper function get_available_cpu_count to safely determine the number of available CPU cores, respecting process affinity, Docker limits, and pytest-xdist workers. This helper is integrated across several modules, including linear_qubit_operator and THC resource estimation utilities, to replace standard multiprocessing.cpu_count() or os.cpu_count() calls. Additionally, a conftest.py fixture is added to limit threadpools when running parallel tests. The reviewer's feedback suggests avoiding unconditionally overwriting the MKL_NUM_THREADS environment variable in thc_factorization.py and thc_objectives.py to prevent overriding explicit user configurations in shared or HPC environments.

[mhucka]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a robust get_available_cpu_count utility to accurately determine available CPU cores while respecting process affinity, Docker container limits, and pytest-xdist worker counts. It integrates this utility across the codebase, replacing standard multiprocessing.cpu_count and os.cpu_count calls, and adds a pytest fixture to limit threadpools during parallel testing. The review feedback suggests extending the environment variable configuration in the THC utilities to also set OMP_NUM_THREADS and OPENBLAS_NUM_THREADS alongside MKL_NUM_THREADS to ensure comprehensive thread limiting across different BLAS backends.

[gemini-code-assist]

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

[arettig]

Looks good! I can confirm it is still very fast on my machine.

A couple small questions about defaulting to parallel vs. serial below. From what I can tell pytest and mypy run in serial by default whereas pylint, and format-incremental run in parallel by default. (Of course this largely doesn't matter for anything except the pytests).

[mhucka]

I thought that mypy didn't have a parallelism option, but now reading the docs, yes, it does have an option for parallel workers. I'll adjust the check/all script to make use of that.

[mhucka]

Regarding format-incremental: it uses black, which defaults to parallel execution.

It's tempting to also change it, to make it consistent (i.e., make it run serially by default), but because it doesn't run any code (unlike pytest), and because I'm not sure what benefit there would be, I'm leaning towards leaving it as-is (parallel).