Skip to content

Fix --raytune-max-concurrent-trials not passed to TuneConfig - #1403

Open
castor639 wants to merge 1 commit into
chemprop:mainfrom
castor639:raytune-max-concurrent-trials
Open

castor639 wants to merge 1 commit into
chemprop:mainfrom
castor639:raytune-max-concurrent-trials

Conversation

@castor639

Copy link
Copy Markdown

The CLI claimed max_concurrent_trials was passed to Ray TuneConfig, but tune_model only used it to divide CPU/GPU resources when num_cpus/gpus were also set. Pass it through TuneConfig so concurrency is honored (e.g. =1 for sequential trials), validate non-positive values, and add a unit regression for #1402.

Description

Include a brief summary of the bug/feature/etc. that this PR seeks to address

Example / Current workflow

Include a sample workflow to either (a) reproduce the bug with current codebase or (b) showcase the deficiency does this PR seeks to address

Bugfix / Desired workflow

Include either (a) the same workflow from above with the correct output produced via this PR (b) some (pseudo)code containing the new workflow that this PR will (seek to) implement

Questions

If there are open questions about implementation strategy or scope of the PR, include them here

Relevant issues

If appropriate, please tag them here and include a quick summary

Checklist

  • linted with flake8?
  • (if appropriate) unit tests added?

The CLI claimed max_concurrent_trials was passed to Ray TuneConfig, but
tune_model only used it to divide CPU/GPU resources when num_cpus/gpus
were also set. Pass it through TuneConfig so concurrency is honored
(e.g. =1 for sequential trials), validate non-positive values, and add
a unit regression for chemprop#1402.
@KnathanM

KnathanM commented Sep 2, 2026

Copy link
Copy Markdown
Member

Hi @castor639, thanks for the PR. Several users are reporting issues using Ray. Have you had success using Ray with Chemprop recently?

@castor639

Copy link
Copy Markdown
Author

No, haven't run a live Ray hpopt recently. This only passes --raytune-max-concurrent-trials into TuneConfig, which the CLI already claimed to do. Doesn't fix the Ray issues in #1394 / #1402.

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