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.

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

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

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.

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
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
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
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@wyli
Copy link
Contributor

wyli commented Aug 21, 2021

/build

monai/utils/nvtx.py Outdated Show resolved Hide resolved
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

Signed-off-by: Behrooz <3968947+drbeh@users.noreply.github.com>
@wyli
Copy link
Contributor

wyli commented Aug 23, 2021

/build

@wyli wyli enabled auto-merge (squash) August 23, 2021 18:49
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.

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

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants