fix(experiment): 🐛 Respect a Joblib launcher pinned to one job - #175
Merged
Conversation
`launcher_is_parallel` classified any non-builtin launcher as concurrent from its `_target_` alone. joblib's `Parallel(n_jobs=1)` selects the sequential backend and runs every job in the calling process, so a run with `hydra/launcher=joblib hydra.launcher.n_jobs=1` was serial on both levels: Hydra ran the jobs one at a time and `batch_run` skipped its parallel branch believing Hydra had it covered. `parallels` was ignored exactly as in #169, only in a narrower configuration. The check now reads `n_jobs` as well. joblib is the only launcher that spells its concurrency knob that way; every other value, the -1 default and an absent key all leave the launcher classified as parallel. An `n_jobs` that cannot be resolved keeps that optimistic default rather than raising out of a predicate and aborting the run. Reported by CodeRabbit on #173.
The job checks out, fetches, and opens a pull request against a branch that already exists on the remote; it never pushes. `contents: read` is enough for the first two and `pull-requests: write` covers the third. Reported by CodeRabbit on #173.
`uv.lock` still recorded abses 0.10.0, left over from the version drift between dev and master. Any `uv run` regenerated it and dirtied the working tree.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
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
1 participant
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.
CodeRabbit 在 #173 上报的两条 review，核实后都成立，一并修掉。
1.
launcher_is_parallel对n_jobs: 1判断错误（主要）launcher_is_parallel只看_target_：只要不是 Hydra 内置的BasicLauncher，就认定为并行。但 joblib 插件可以被配置成串行——Parallel(n_jobs=1)走的是 sequential backend，在调用进程里跑完所有任务，实测：所以
hydra/launcher=joblib hydra.launcher.n_jobs=1会两层都串行：Hydra 逐个跑 job，batch_run又以为 Hydra 在并行而跳过自己的并行分支，parallels被忽略——和 #169 是同一个失效模式，只是配置更窄。修复：判断时把
n_jobs也读进来。joblib 是唯一用这个名字的 launcher（submitit 用tasks_per_node，ray 有自己的一套），所以这一条判断不会误伤别人。行为表：n_jobs-1（插件默认值，实测确认）41???/null最后一行是我自己引入的边界：
OmegaConf.select碰到解析不了的插值会抛InterpolationKeyError，而这是通往batch_run的一个谓词，在这里抛异常会中断整个实验。谓词应当是全函数，所以兜底为乐观的 parallel——反正代价只是少嵌套一层。严重程度：CodeRabbit 标的是 🟠 Major，我觉得偏 Minor。要触发得显式覆写
n_jobs=1（插件默认是-1），后果是并行度丢失而不是结果错误。2. back-merge job 权限过宽
backmerge-dev只做 checkout / fetch /gh pr create，--head master用的是远端已有分支，全程不 push。contents: write降为contents: read。3. 顺带：
uv.lock版本漂移uv.lock里 abses 还记着0.10.0，是 dev/master 版本漂移的残留，导致每次uv run都会把工作区弄脏。同步到0.11.7。验证
709 passed合并后 #173 会自动更新。