Skip to content
New issue

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

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

Already on GitHub? # to your account

Implement several fixes and QoL improvements to the Data Preparation MLCube #85

Open
wants to merge 53 commits into
base: fets_2.0
Choose a base branch
from

Conversation

aristizabal95
Copy link

Proposed Changes

  • Provide methods for testing the MLCube with simpler models
  • Provide more verbose stdout regarding subject stage to run
  • Fix issues related to report file writes
  • Handle possible infinite loop when rerunning the pipeline if changes happened
  • Handle memory leaks by running affected external code in a separate process
  • Use a version of pytorch with a more compatible CUDA version
  • requirements versioning
    And many more changes.

aristizabal95 and others added 30 commits January 19, 2024 14:25
Move model execution to subprocesses to eliminate memleak
Wrap conflicting functions instead of stages
Copy link
Member

@sarthakpati sarthakpati left a comment

Choose a reason for hiding this comment

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

Minor comments (only white space additions)

models
tmpmodel
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
tmpmodel
tmpmodel

DIFF=$(echo "$ENDTIME - $STARTTIME" | bc)
echo $DIFF

run_other
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
run_other
run_other

DIFF=$(echo "$ENDTIME - $STARTTIME" | bc)
echo $DIFF

run_other
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
run_other
run_other

RUN cp -r /project/tmpmodel /project/stages/data_prep_models/brain_extraction/model_0
RUN mv /project/tmpmodel /project/stages/data_prep_models/brain_extraction/model_1

ENTRYPOINT ["python", "/project/mlcube.py"]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ENTRYPOINT ["python", "/project/mlcube.py"]
ENTRYPOINT ["python", "/project/mlcube.py"]

report = init_report(args)
pipeline = init_pipeline(args)
pipeline.run(report, args.report)

# cleanup tmp folder
shutil.rmtree(tmpfolder, ignore_errors=True)

if __name__ == "__main__":
main()
Copy link
Member

Choose a reason for hiding this comment

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

Please add a new line at the end for consistency.

)

if __name__ == "__main__":
main()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
main()
main()

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants