Skip to content
This repository was archived by the owner on Mar 21, 2024. It is now read-only.

BUG: Dont update multi-node env vars for single node training#796

Merged
mebristo merged 4 commits intomainfrom
mebristo/single_node_training
Sep 2, 2022
Merged

BUG: Dont update multi-node env vars for single node training#796
mebristo merged 4 commits intomainfrom
mebristo/single_node_training

Conversation

@mebristo
Copy link
Copy Markdown
Contributor

@mebristo mebristo commented Sep 1, 2022

Environemnt variables seem to be getting set in Azure ML for single node jobs that cause our logic to fail in set_environment_variables_for_multi_node therefore we should avoid calling this for jobs on 1 node

@mebristo mebristo changed the title Dont update multi-node env vars for single node training FIX: Dont update multi-node env vars for single node training Sep 2, 2022
@ant0nsc
Copy link
Copy Markdown
Contributor

ant0nsc commented Sep 2, 2022

@mebristo, can we not use the updated function from hi-ml? Just publish a new package version and consume it here.

@mebristo
Copy link
Copy Markdown
Contributor Author

mebristo commented Sep 2, 2022

I sent you a message about this offline - the hi-ml package hasn't been updated in a while, so updating it in this PR would also require refactoring how users can provide conda files (since merge_conda_files has been removed from hi-ml in favour of a single command line arg) ... so in the spirit of a 'quick fix' to unblock PRs I felt this was the best approach

@mebristo mebristo changed the title FIX: Dont update multi-node env vars for single node training BUG: Dont update multi-node env vars for single node training Sep 2, 2022
@mebristo mebristo marked this pull request as ready for review September 2, 2022 16:01
@mebristo mebristo requested a review from fepegar September 2, 2022 16:03
@mebristo mebristo enabled auto-merge (squash) September 2, 2022 16:06
@mebristo mebristo merged commit 8cf63c8 into main Sep 2, 2022
@mebristo mebristo deleted the mebristo/single_node_training branch September 2, 2022 16:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants