Skip to content

Rebase of #1737 (libE service mode) onto develop + comms=local pickling fix - #1794

Open
jlnav wants to merge 13 commits into
developfrom
fix/service-mode-rebase
Open

jlnav wants to merge 13 commits into
developfrom
fix/service-mode-rebase

Conversation

@jlnav

@jlnav jlnav commented Oct 15, 2026

Copy link
Copy Markdown
Member

Summary

This is a rebase of @shuds13 PR #1737 ("Add libE service mode") onto current develop, opened as a separate PR (rather than pushing to the feature/libe_service_mode branch) to avoid surprising him with new activity there. Happy to close this in favor of him updating #1737 directly if preferred.

#1737 had gone stale and diverged significantly from develop. While rebasing and testing it, I found and fixed one additional bug beyond the manager hang already fixed on develop in a5dd111 (which this branch inherits via the merge):

QueueGenerator pickling under comms="local"

QueueGenerator/QueueService (libensemble/gen_classes/queue_gen.py) stored live queue.Queue objects (and QueueService additionally a threading.Thread) as plain instance attributes. Under comms="local" (multiprocessing with spawn, the default on macOS/Windows and the default everywhere as of Python 3.14), gen_specs -- including the generator instance -- gets pickled for every spawned worker process, even sim-only ones. queue.Queue holds a threading.Lock, which is unpicklable under spawn, causing all three of this PR's own regression tests to fail 100% of the time under comms="local".

Fixed by:

  • Adding QueueGenerator.__getstate__ to strip input_queue/output_queue before pickling (the generator always runs on the manager; sim-only workers never need these).
  • Adding an assert not getattr(libE_specs, "gen_on_worker", False) guard in QueueService.__init__, since this generator's queues are only valid in the manager's own process/thread.

Changes

  • Merge of develop into the original feature/libe_service_mode branch (resolved a doc conflict in libE_specs_general.rst and a duplicate-fix conflict in persistent_support.py).
  • libensemble/gen_classes/queue_gen.py: pickling fix described above.

Testing

  • test_queue_gen.py, test_queue_gen_streaming.py, test_queue_gen_idle_timeout.py (this PR's own new regression tests) all pass under comms="local".
  • test_manager_main.py unit tests pass.
  • black/flake8/pre-commit clean.

Depends on / overlaps with #1793 (regression test for the manager hang bug, targeted at develop separately since that fix already lives there).

shuds13 and others added 10 commits May 25, 2026 12:46
…ebase

# Conflicts:
#	docs/data_structures/libE_specs/libE_specs_general.rst
#	libensemble/tools/persistent_support.py
The live queue.Queue objects held by QueueGenerator can't be pickled once
libE spawns sim-only worker processes under comms=local (multiprocessing's
spawn start method - always on macOS/Windows, and the default everywhere
as of Python 3.14). gen_specs, including the generator instance, is pickled
for every sim worker regardless of whether it ever runs the generator.

QueueGenerator.__getstate__ now drops the live queues before pickling, since
sim-only workers never use them (the generator always runs on the manager
by default). QueueService also now asserts gen_on_worker is not set, since
this generator's queues are only valid in the manager's process/thread.
QueueService already asserted gen_on_worker=False, but QueueGenerator is
documented for direct use too (bypassing that guard). Add a _check_queues
guard in suggest()/ingest() so a generator that ends up on a worker process
(losing its queues via __getstate__) fails with a clear RuntimeError instead
of a confusing AttributeError. Add unit tests covering both paths.
@jlnav

jlnav commented Oct 15, 2026

Copy link
Copy Markdown
Member Author

Good catch from offline discussion: gen_on_worker is a real feature (generators don't always run on the manager), and my earlier description overstated that. QueueService already asserted gen_on_worker=False, but QueueGenerator is documented for direct use too (see its module docstring), which bypassed that guard entirely.

Pushed a follow-up commit (607cbd9) that adds a _check_queues guard in QueueGenerator.suggest()/ingest(), so if the generator ever ends up running somewhere other than the manager (queues stripped to None via __getstate__), it fails with a clear RuntimeError instead of a confusing AttributeError. Added unit tests in libensemble/tests/unit_tests/test_queue_gen.py covering both the QueueService guard and the direct-use guard.

@codecov

codecov Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.48485% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.99%. Comparing base (6753d43) to head (018d57a).
⚠️ Report is 8 commits behind head on develop.

Files with missing lines Patch % Lines
libensemble/gen_classes/queue_gen.py 99.12% 0 Missing and 1 partial ⚠️
libensemble/manager.py 92.85% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1794      +/-   ##
===========================================
+ Coverage    88.77%   88.99%   +0.21%     
===========================================
  Files           73       74       +1     
  Lines         8658     8793     +135     
  Branches      1402     1420      +18     
===========================================
+ Hits          7686     7825     +139     
+ Misses         725      721       -4     
  Partials       247      247              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jlnav
jlnav added this pull request to stack #1796 October 16, 2026 16:26
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