[Fix] Move openmpi_oversubscribe to init function rather than resource_dict

[jan-janssen]

Summary by CodeRabbit

Release Notes

• New Features
  • All executor types (Flux, SLURM, single-node, and test cluster) now support configurable OpenMPI oversubscribe settings. Users can enable or disable job oversubscription per executor, providing greater flexibility for resource-constrained environments. This parameter defaults to disabled and can be specified during executor initialization.

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request adds a new openmpi_oversubscribe parameter (boolean, default False) to executor constructors across three modules: FluxJobExecutor, FluxClusterExecutor, SingleNodeExecutor, TestClusterExecutor, SlurmClusterExecutor, and SlurmJobExecutor. The parameter is propagated into default_resource_dict configurations, replacing hardcoded False values with the provided argument.

Changes

Cohort / File(s)	Summary
Executor Parameter Addition src/executorlib/executor/flux.py, src/executorlib/executor/single.py, src/executorlib/executor/slurm.py	Added openmpi_oversubscribe parameter to all executor constructors with default value False; propagated the value into default_resource_dict to replace hardcoded defaults; updated docstrings and method signatures to reflect the new configuration option.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• Fix DocString in SingleNodeExecutor #722 — Modifies SingleNodeExecutor constructor signature by adding a different boolean parameter, affecting the same executor class.
• Enforce manual backend selection #399 — Adds and propagates the same openmpi_oversubscribe parameter through executor constructors and default resource dictionaries, targeting identical configuration surfaces.

Poem

🐰 A parameter hops through three executor files,
Configuration spreads across single, slurm, and flux with smiles,
Oversubscribe no longer hardcoded and rigid,
Flexibility blooms where defaults once stood rigid! 🌱

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: moving openmpi_oversubscribe from resource_dict to init function parameters across multiple executor classes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch openmpi_oversubscribe

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.89%. Comparing base (2a94f49) to head (62eb9f5).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #937   +/-   ##
=======================================
  Coverage   93.89%   93.89%           
=======================================
  Files          38       38           
  Lines        1981     1981           
=======================================
  Hits         1860     1860           
  Misses        121      121           

☔ View full report in Codecov by Sentry.
📢 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.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/executorlib/executor/flux.py`:
- Line 73: The docstring for the openmpi_oversubscribe parameter is misleading
because create_flux_executor calls check_oversubscribe() which raises a
ValueError when True; update the docstrings on FluxJobExecutor and
FluxClusterExecutor (where openmpi_oversubscribe is documented) to either remove
the claim that it "adds the `--oversubscribe` command flag" or explicitly state
that openmpi_oversubscribe is unsupported for the Flux backend and will raise an
error via check_oversubscribe(); reference create_flux_executor and
check_oversubscribe in your change so readers know why the parameter cannot be
True.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the check_oversubscribe function definition and implementation
rg -n -B 2 -A 15 'def check_oversubscribe' src/executorlib/

Repository: pyiron/executorlib

Length of output: 1369

---

🏁 Script executed:

#!/bin/bash
# Look at the create_flux_executor function to confirm key deletion
rg -n -B 5 -A 20 'def create_flux_executor' src/executorlib/executor/flux.py

Repository: pyiron/executorlib

Length of output: 1116

---

🏁 Script executed:

#!/bin/bash
# Check the docstring at lines 73 and 275 to see the exact wording
sed -n '60,85p' src/executorlib/executor/flux.py
sed -n '260,290p' src/executorlib/executor/flux.py

Repository: pyiron/executorlib

Length of output: 3918

---

🏁 Script executed:

#!/bin/bash
# Get the full create_flux_executor implementation
sed -n '453,600p' src/executorlib/executor/flux.py

Repository: pyiron/executorlib

Length of output: 6435

---

🏁 Script executed:

#!/bin/bash
# Also check where create_flux_executor is called from FluxJobExecutor
rg -n -B 3 -A 3 'create_flux_executor' src/executorlib/executor/flux.py | head -40

Repository: pyiron/executorlib

Length of output: 1317

---

Remove or clarify openmpi_oversubscribe docstring for Flux executors — parameter causes runtime error

The docstring claims the parameter "adds the --oversubscribe command flag (OpenMPI and SLURM)", but create_flux_executor calls check_oversubscribe() which raises a ValueError if the value is True:

"Oversubscribing is not supported for the executorlib.flux.PyFLuxExecutor backend. 
Please use oversubscribe=False instead of oversubscribe=True."

This affects both FluxJobExecutor and FluxClusterExecutor. The docstring (at lines 73 and 275) should either be removed or clarified to note that this parameter is unsupported on the Flux backend and will raise an error if set to True.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/executorlib/executor/flux.py` at line 73, The docstring for the
openmpi_oversubscribe parameter is misleading because create_flux_executor calls
check_oversubscribe() which raises a ValueError when True; update the docstrings
on FluxJobExecutor and FluxClusterExecutor (where openmpi_oversubscribe is
documented) to either remove the claim that it "adds the `--oversubscribe`
command flag" or explicitly state that openmpi_oversubscribe is unsupported for
the Flux backend and will raise an error via check_oversubscribe(); reference
create_flux_executor and check_oversubscribe in your change so readers know why
the parameter cannot be True.