Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BUG: Stop looking for environment file if already running in AML #432

Merged
merged 3 commits into from
Jun 13, 2022
Merged

BUG: Stop looking for environment file if already running in AML #432

merged 3 commits into from
Jun 13, 2022

Conversation

mebristo
Copy link
Member

Currently, if the user specifies a relative path as --conda_env, it will be found by the first process which will then start training, and change the cwd to 'outputs'. This means that spawned processes can't find the relative conda_env path. This PR enables spawned processes to still find the conda env path

@mebristo mebristo changed the title FIX: hotfix to enable relative conda envs to be found by spawned processes FIX: Hotfix to enable relative conda envs to be found by spawned processes Jun 10, 2022
@mebristo mebristo changed the title FIX: Hotfix to enable relative conda envs to be found by spawned processes FIX: Hotfix, enable relative envs to be found by spawned processes Jun 10, 2022
@codecov
Copy link

codecov bot commented Jun 10, 2022 •
edited
Loading

Codecov Report

Merging #432 (fc7e731) into main (784ea90) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

Flag Coverage Δ
hi-ml 80.07% <100.00%> (ø)
hi-ml-histopathology 79.06% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
hi-ml/src/health_ml/runner.py 80.68% <100.00%> (ø)
...istopathology/src/histopathology/models/deepmil.py 85.16% <0.00%> (ø)
...-histopathology/src/histopathology/utils/naming.py 98.59% <0.00%> (+0.02%) ⬆️

@mebristo mebristo changed the title FIX: Hotfix, enable relative envs to be found by spawned processes BUG: Hotfix, enable relative envs to be found by spawned processes Jun 10, 2022
ant0nsc
ant0nsc requested changes Jun 10, 2022
View reviewed changes
Copy link
Collaborator

@ant0nsc ant0nsc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can be achieved simpler, please see my comment.

hi-ml/src/health_ml/runner.py Outdated Show resolved Hide resolved
@ant0nsc ant0nsc mentioned this pull request Jun 13, 2022
Closed
@mebristo mebristo changed the title BUG: Hotfix, enable relative envs to be found by spawned processes BUG: Hotfix, don't look for environment file if already running in AML Jun 13, 2022
@mebristo mebristo requested a review from fepegar June 13, 2022 09:24
@fepegar fepegar changed the title BUG: Hotfix, don't look for environment file if already running in AML BUG: Stop looking for environment file if already running in AML Jun 13, 2022
fepegar
fepegar approved these changes Jun 13, 2022
View reviewed changes
Copy link
Contributor

@fepegar fepegar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I edited the PR title for consistency, I hope that's ok.

@mebristo mebristo merged commit 746c8b5 into main Jun 13, 2022
@mebristo mebristo deleted the mebristo/hotfix_relative_conda_env branch June 13, 2022 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ant0nsc ant0nsc ant0nsc approved these changes

@fepegar fepegar fepegar approved these changes

@kenza-bouzid kenza-bouzid Awaiting requested review from kenza-bouzid

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mebristo @ant0nsc @fepegar