Skip to content

Lazy functionality 1 2#6537

Merged
Nic-Ma merged 210 commits intoProject-MONAI:devfrom
atbenmurray:lazy_functionality_1_2
Jun 1, 2023
Merged

Lazy functionality 1 2#6537
Nic-Ma merged 210 commits intoProject-MONAI:devfrom
atbenmurray:lazy_functionality_1_2

Conversation

@atbenmurray
Copy link
Copy Markdown
Contributor

@atbenmurray atbenmurray commented May 22, 2023 •
edited
Loading

Reduced lazy resampling functionality for MONAI 1.2

Description

This PR is a subset of #6257 intended for MONAI 1.2. It contains the basic resampling strategy that has been approved for the 1.2 release during MONAI core dev meeting of 19th May, 2023.

Draft status:

  • still to do
    • doc strings
    • topic page
    • resolve compose reference doc issue

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

atbenmurray and others added 30 commits March 27, 2023 22:43
@atbenmurray
Initial commit
06d5842 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Refactors and simplification of api / calls
2507a0b 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Refactoring functional spatial transform to support lazy_evaluation p…
56b618e 
…arameter, refactoring spatial arrays to support lazy_evaluation call time parameter, refactoring _apply_transform to pass call time lazy_evaluation flag

Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Making lazy_evaluation parameter consistent across spatial array lazy…
315e5b7 
… transforms

Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Updating apply and dictionary spatial transforms to support
4e69313 
lazy_evaluation initing / calls

Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Missing changes to spatial dictionary
99fa35b 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Enable call-time overriding of lazy_evaluation in croppad/functional and
a798e0e 
croppad/array

Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Updating croppad dictionary and croppad array for call-time laziness
114b8cc 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Fix for push_transform; updating the lazy integration test
5df4261 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
removing issue.txt
84d65f7 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
. Renaming apply_transform to apply_pending as it is more semantically
7ec442f 
meaningful
. Adding ApplyPending and ApplyPendingd transforms

Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Tweaked array ApplyPending and ApplyPendingd for array and dict data
8f62776 
respectively

Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Added missing license text.
2492223 
Filter for metatensors in ApplyPending / ApplyPendingd classes

Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Missing import for apply_pending
0064326 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Merge branch 'dev' into lazy_eval_refactor
0646602 
Signed-off-by: Ben Murray <[email protected]>
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
05eb3f9 
for more information, see https://pre-commit.ci
@atbenmurray
Fixing syntax error created by merge of dev to PR
788cd74 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Fixing issues with pad_func post merge
4bd22f8 
Signed-off-by: Ben Murray <[email protected]>
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
1e0510c 
for more information, see https://pre-commit.ci
@atbenmurray
Fixing issues in crop while running unit tests
c36b69f 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Merge branch 'lazy_eval_refactor' of github.com:atbenmurray/monai int…
8a4f310 
…o lazy_eval_refactor
@atbenmurray
Fixing issues with OneOf/SomeOf/RandomOrder after dev merge
3c69776 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
More croppad fixes post dev merge
6b94aa1 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Fixing test_nvtx_decorator after def merge
bf1c01a 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Removed use of **kwargs parameter in tests as it isn't supported by a…
19745f9 
…pply_pending. Fixing various tests by moving to use of 'overrides' on apply_pending

Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Setting use of lazy_evaluation to False for Rand2D/Rand3DElastic
b219f13 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Fixed test_spatial_pad
6af8f24 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Renaming lazy_evaluation to lazy
173c161 
Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Fixed all outstanding usages of apply_pending in tests. Removed the
2262ba9 
**kwargs parameter from apply_pending

Signed-off-by: Ben Murray <[email protected]>
@atbenmurray
Merge branch 'dev' into lazy_eval_refactor
aaaea13 
Signed-off-by: Ben Murray <[email protected]>
ericspod
ericspod reviewed May 30, 2023
View reviewed changes
@ericspod ericspod self-requested a review May 30, 2023 11:37
ericspod
ericspod approved these changes May 30, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@ericspod ericspod left a comment

Choose a reason for hiding this comment

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

I'm good with it now, we need to rerun tests with the current dev merged in. @Nic-Ma? Thanks!

Nic-Ma
Nic-Ma approved these changes May 30, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@Nic-Ma Nic-Ma left a comment

Choose a reason for hiding this comment

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

Thanks for the quick update, put some comments inline, others look good to me now.

@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented May 30, 2023

Hi @KumoLiu ,

The functionality of this PR is ready, just some minor comments left.
Could you please help quickly run the benchmark tutorials with this PR to verify the performance for MONAI 1.2 release?

Thanks in advance.

@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented May 30, 2023

Hi @mingxin-zheng , @dongyang0122 , @wyli ,

I see some Auto3D failures in the CI results of this PR, could you please help confirm whether they are related to this PR?
@atbenmurray Can you run these tests locally to verify?

======================================================================
ERROR: test_get_history (tests.test_auto3dseg_hpo.TestHPO)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/__w/MONAI/MONAI/tests/test_auto3dseg_hpo.py", line 180, in test_get_history
    NNIGen().run_algo(obj_filename, self.work_dir)
  File "/__w/MONAI/MONAI/monai/apps/auto3dseg/hpo_gen.py", line 231, in run_algo
    self.algo.train(self.params)
  File "/__w/MONAI/MONAI/monai/apps/auto3dseg/bundle_gen.py", line 269, in train
    return self._run_cmd(cmd)
  File "/__w/MONAI/MONAI/monai/apps/auto3dseg/bundle_gen.py", line 246, in _run_cmd
    return subprocess.run(cmd_list, env=ps_environ, check=True)
  File "/opt/conda/lib/python3.8/subprocess.py", line 516, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['torchrun', '--nnodes=1', '--nproc_per_node=2', '/tmp/tmp51lklfjx/workdir/swinunetr_0_override_None/scripts/train.py', 'run', "--config_file='/tmp/tmp51lklfjx/workdir/swinunetr_0_override_None/configs/hyper_parameters.yaml','/tmp/tmp51lklfjx/workdir/swinunetr_0_override_None/configs/network.yaml','/tmp/tmp51lklfjx/workdir/swinunetr_0_override_None/configs/transforms_infer.yaml','/tmp/tmp51lklfjx/workdir/swinunetr_0_override_None/configs/transforms_train.yaml','/tmp/tmp51lklfjx/workdir/swinunetr_0_override_None/configs/transforms_validate.yaml'"]' returned non-zero exit status 1.

======================================================================
ERROR: test_run_algo (tests.test_auto3dseg_hpo.TestHPO)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/__w/MONAI/MONAI/tests/test_auto3dseg_hpo.py", line 148, in test_run_algo
    NNIGen().run_algo(obj_filename, self.work_dir)
  File "/__w/MONAI/MONAI/monai/apps/auto3dseg/hpo_gen.py", line 231, in run_algo
    self.algo.train(self.params)
  File "/__w/MONAI/MONAI/monai/apps/auto3dseg/bundle_gen.py", line 269, in train
    return self._run_cmd(cmd)
  File "/__w/MONAI/MONAI/monai/apps/auto3dseg/bundle_gen.py", line 246, in _run_cmd
    return subprocess.run(cmd_list, env=ps_environ, check=True)
  File "/opt/conda/lib/python3.8/subprocess.py", line 516, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['torchrun', '--nnodes=1', '--nproc_per_node=2', '/tmp/tmpe1a9xrrd/workdir/swinunetr_0_override_None/scripts/train.py', 'run', "--config_file='/tmp/tmpe1a9xrrd/workdir/swinunetr_0_override_None/configs/hyper_parameters.yaml','/tmp/tmpe1a9xrrd/workdir/swinunetr_0_override_None/configs/network.yaml','/tmp/tmpe1a9xrrd/workdir/swinunetr_0_override_None/configs/transforms_infer.yaml','/tmp/tmpe1a9xrrd/workdir/swinunetr_0_override_None/configs/transforms_train.yaml','/tmp/tmpe1a9xrrd/workdir/swinunetr_0_override_None/configs/transforms_validate.yaml'"]' returned non-zero exit status 1.

======================================================================
ERROR: test_run_optuna (tests.test_auto3dseg_hpo.TestHPO)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/__w/MONAI/MONAI/tests/test_auto3dseg_hpo.py", line 163, in test_run_optuna
    study.optimize(
  File "/opt/conda/lib/python3.8/site-packages/optuna/study/study.py", line 443, in optimize
    _optimize(
  File "/opt/conda/lib/python3.8/site-packages/optuna/study/_optimize.py", line 66, in _optimize
    _optimize_sequential(
  File "/opt/conda/lib/python3.8/site-packages/optuna/study/_optimize.py", line 163, in _optimize_sequential
    frozen_trial = _run_trial(study, func, catch)
  File "/opt/conda/lib/python3.8/site-packages/optuna/study/_optimize.py", line 251, in _run_trial
    raise func_err
  File "/opt/conda/lib/python3.8/site-packages/optuna/study/_optimize.py", line 200, in _run_trial
    value_or_values = func(trial)
  File "/__w/MONAI/MONAI/monai/apps/auto3dseg/hpo_gen.py", line 336, in __call__
    self.run_algo(obj_filename, output_folder, template_path)
  File "/__w/MONAI/MONAI/monai/apps/auto3dseg/hpo_gen.py", line 394, in run_algo
    self.algo.train(self.params)
  File "/__w/MONAI/MONAI/monai/apps/auto3dseg/bundle_gen.py", line 269, in train
    return self._run_cmd(cmd)
  File "/__w/MONAI/MONAI/monai/apps/auto3dseg/bundle_gen.py", line 246, in _run_cmd
    return subprocess.run(cmd_list, env=ps_environ, check=True)
  File "/opt/conda/lib/python3.8/subprocess.py", line 516, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['torchrun', '--nnodes=1', '--nproc_per_node=2', '/tmp/tmpx65fltgg/optuna_test/swinunetr_0_override_learning_rate_0.01/scripts/train.py', 'run', "--config_file='/tmp/tmpx65fltgg/optuna_test/swinunetr_0_override_learning_rate_0.01/configs/hyper_parameters.yaml','/tmp/tmpx65fltgg/optuna_test/swinunetr_0_override_learning_rate_0.01/configs/network.yaml','/tmp/tmpx65fltgg/optuna_test/swinunetr_0_override_learning_rate_0.01/configs/transforms_infer.yaml','/tmp/tmpx65fltgg/optuna_test/swinunetr_0_override_learning_rate_0.01/configs/transforms_train.yaml','/tmp/tmpx65fltgg/optuna_test/swinunetr_0_override_learning_rate_0.01/configs/transforms_validate.yaml'", '--learning_rate=0.01']' returned non-zero exit status 1.

----------------------------------------------------------------------
Ran 13507 tests in 1115.695s

FAILED (errors=3, skipped=911)

Thanks in advance.

@mingxin-zheng
Copy link
Copy Markdown
Contributor

mingxin-zheng commented May 31, 2023 •
edited
Loading

Hi @Nic-Ma @atbenmurray , looks like the error is generated by this PR. I ran it on my local machine and the error log indicates the causes are auto3dseg templates use lazy_evaluation:

TypeError: __init__() got an unexpected keyword argument 'lazy_evaluation'

https://github.com/Project-MONAI/research-contributions/blob/fddf97ac09a176d9df4c4369cd07fe176b1c4eb4/auto3dseg/algorithm_templates/swinunetr/configs/transforms_train.yaml#L5

I think it is an expected failure and can be addressed in another PR.

@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented May 31, 2023

Hi @dongyang0122 ,

Could you please help update the Auto3DSeg templates accordingly to avoid any lazy resampling logic?

Thanks in advance.

@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Jun 1, 2023

/black

@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Jun 1, 2023

/build

@monai-bot
[MONAI] code formatting
0d4143c 
Signed-off-by: monai-bot <[email protected]>
@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Jun 1, 2023

/build

@atbenmurray
Copy link
Copy Markdown
Contributor Author

atbenmurray commented Jun 1, 2023

@ericspod @Nic-Ma @wyli Are we ready to merge then?

@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Jun 1, 2023

/black

@Nic-Ma
Copy link
Copy Markdown
Contributor

Nic-Ma commented Jun 1, 2023

/build

@ericspod
Copy link
Copy Markdown
Member

ericspod commented Jun 1, 2023 via email

@Nic-Ma Nic-Ma enabled auto-merge (squash) June 1, 2023 09:54
@Nic-Ma Nic-Ma merged commit e597994 into Project-MONAI:dev Jun 1, 2023
This was referenced Jun 9, 2023
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Nic-Ma Nic-Ma Nic-Ma approved these changes

@ericspod ericspod ericspod approved these changes

@wyli wyli Awaiting requested review from wyli

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@atbenmurray @Nic-Ma @mingxin-zheng @ericspod @monai-bot