Issue: #791 Add args and check condition for remote GPU

[Zhiyuan-w]

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes #

Type of change

What types of changes does your code introduce to DeepReg?

Please check the boxes that apply after submitting the pull request.

• [ v] Bugfix (non-breaking change which fixes an issue)
• [v ] Code style update (formatting, renaming)
• [ ] Refactoring (no functional changes, no api changes)
• [ ] Documentation Update (fix or improvement on the documentation)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Other (if none of the other choices apply)

Checklist

Please check the boxes that apply after submitting the pull request.

If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

• [v ] I have installed pre-commit using pre-commit install and formatted all changed files. If you are not certain, run pre-commit run --all-files.
• [v ] My commits' message styles matches our requested structure, e.g. Issue #<issue number>: detailed message.
• [ ] I have updated the change log file regarding my changes.
• [ ] I have added tests that prove my fix is effective or that my feature works
• [ v] I have added necessary documentation (if appropriate)

[Zhiyuan-w]

It may cause a SyntaxError: non-default argument follows default argument. As I do:

    gpu: str=None in train, predict

Could I keep the "--gpu" as required and add a "--remote" arg? Like

"--remote",        "-r",          help="Use GPU remotely for training.",           dest="remote",          action="store_true",  
     required=False,       default=False

In train()

train(....., remote:bool = False)
if not remote:
       os.environ["CUDA_VISIBLE_DEVICES"] = gpu        
       os.environ["TF_FORCE_GPU_ALLOW_GROWTH"] = "true" if gpu_allow_growth else "false"

My wifi is not stable right now. Sorry for sending comments twice.

[NMontanaBrown]

@Zhiyuan-w Hi, lets not add more arguments to CLI if not absolutely necessary. I still think we can configure via gpu argument. Let me have a think

[NMontanaBrown]

@Zhiyuan-w ok so instead of making gpu:str = None in the functions, just make the default arg in GPU CLI None or False. That way, the same logic applies but we don't need to change the order of the args or add remote to the CLI. So simply passing "GPU" arg to CLI will trigger CUDA_VISIBLE_DEVICES, if it is not passed it will be put in as "False". Does that make sense?

[Zhiyuan-w]

Good idea. I will push the new commit today.

[codecov[bot]]

Codecov Report

Merging #792 (e31cf5a) into main (35be9fa) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #792   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           41        41           
  Lines         2501      2503    +2     
=========================================
+ Hits          2501      2503    +2     

Impacted Files	Coverage Δ
deepreg/predict.py	100.00% <100.00%> (ø)
deepreg/train.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 35be9fa...e31cf5a. Read the comment docs.

[NMontanaBrown]

Changelog needs to be updated before merge.

[NMontanaBrown]

@mathpluscode @YipengHu can you review please?

[NMontanaBrown]

@Zhiyuan-w Can you merge main into branch so that it is up to date?

[NMontanaBrown]

@Zhiyuan-w Can you merge main into branch so that it is up to date?

BUMP @Zhiyuan-w

[NMontanaBrown]

Thanks for your contribution! @Zhiyuan-w , also @alkististav, check the fix!