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

NVTX Range universal decorator #2814

Merged
merged 23 commits into from
Aug 24, 2021
Merged

NVTX Range universal decorator #2814

merged 23 commits into from
Aug 24, 2021

Conversation

drbeh
Copy link
Member

@drbeh drbeh commented Aug 20, 2021 •
edited
Loading

Description

This PR implements Range which is both a decorator and a context manager for adding NVTX range to most of the MONAI components. It decorates the appropriate method of MONAI components objects such as transfoms, networks, losses, etc.
It also can be used with with runtime context to annotate an arbitrary part of the code.

This PR also removes Range transform in favor of Range decorator to have only one way of doing this. However, the rest of NVTX transforms (Mark, RangePush, RangePop) still have their own use.

Status

Ready

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.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

drbeh added 11 commits August 18, 2021 16:50
@drbeh
Implement NVTX Range decorator and context manager
f565fad 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh
Implement unittests for nvtx range
47c80ac 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh
Update utils init
462fb9a 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh
Remove Range transform in favor of Range decorator
1fb6770 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh
Update docs
21ba71c 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh
Update docstrings
479f550 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh
Merge branch 'dev' of github.com:Project-MONAI/MONAI into range-decor…
6599996 
…ator
@drbeh
Update description
0a4cefe 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh
Remove optional
9a49509 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh
Merge branch 'dev' into range-decorator
d11cf7e
@drbeh
Change torch testing to numpy testing
d609ea8 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh drbeh requested review from Nic-Ma and wyli and removed request for wyli and Nic-Ma August 20, 2021 13:54
@drbeh drbeh marked this pull request as draft August 20, 2021 14:22
@drbeh
Handle special methods
f5180a6 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh drbeh marked this pull request as ready for review August 20, 2021 19:11
@drbeh drbeh requested review from Nic-Ma and wyli August 20, 2021 19:11
drbeh added 2 commits August 20, 2021 19:16
@drbeh
Change typing
7a57fec 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh
REmove torch.nn
3e32875 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@wyli
Copy link
Contributor

wyli commented Aug 21, 2021

/build

wyli
wyli reviewed Aug 21, 2021
View reviewed changes
monai/utils/nvtx.py Outdated Show resolved Hide resolved
@drbeh drbeh mentioned this pull request Aug 23, 2021
Closed
wyli
wyli approved these changes Aug 23, 2021
View reviewed changes
Copy link
Contributor

@wyli wyli left a comment

Choose a reason for hiding this comment

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

thank you, please resolve the conversation then I'll trigger the tests and merge

drbeh added 2 commits August 23, 2021 18:10
@drbeh
Update tests
69b9aae 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh
Merge branch 'dev' into range-decorator
6d24119
@wyli
Copy link
Contributor

wyli commented Aug 23, 2021

/build

@wyli wyli enabled auto-merge (squash) August 23, 2021 18:49
@drbeh
Change _call_impl to __call__
3f5928f 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@wyli
Copy link
Contributor

wyli commented Aug 23, 2021

/build

@drbeh drbeh disabled auto-merge August 23, 2021 18:53
@drbeh
Copy link
Member Author

drbeh commented Aug 23, 2021

Wait a moment. I'm doing minor changes.

@drbeh
Remove debugging outputs
04c42f5 
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@drbeh
Copy link
Member Author

drbeh commented Aug 23, 2021

/build

@drbeh drbeh enabled auto-merge (squash) August 23, 2021 18:57
@wyli
Copy link
Contributor

wyli commented Aug 23, 2021

/build

@wyli wyli disabled auto-merge August 23, 2021 20:14
@wyli wyli enabled auto-merge (squash) August 23, 2021 20:15
@drbeh
Copy link
Member Author

drbeh commented Aug 23, 2021

@wyli, @madil90, do you know why blossom-ci is failing here? Thanks

@wyli
Copy link
Contributor

wyli commented Aug 23, 2021 •
edited
Loading

it's #2821 according to the logs, I try to reproduce locally but it's not easy

@wyli
Copy link
Contributor

wyli commented Aug 23, 2021

/build

@drbeh
Copy link
Member Author

drbeh commented Aug 24, 2021

/build

1 similar comment
@wyli
Copy link
Contributor

wyli commented Aug 24, 2021

/build

@wyli wyli merged commit 42ad892 into Project-MONAI:dev Aug 24, 2021
@drbeh drbeh deleted the range-decorator branch August 24, 2021 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wyli wyli wyli approved these changes

@Nic-Ma Nic-Ma Awaiting requested review from Nic-Ma

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

Successfully merging this pull request may close these issues.
2 participants
@drbeh @wyli