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

name to path changes in configuration #2781

Merged
merged 2 commits into from
Aug 9, 2024
Merged

name to path changes in configuration #2781

merged 2 commits into from
Aug 9, 2024

Conversation

yhwen
Copy link
Collaborator

@yhwen yhwen commented Aug 9, 2024 •
edited by chesterxgchen
Loading

Description

name to path changes due to app_opt scan changes
Update the config changes because we won't scan the app_opt package any more.

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.
  • Quick tests passed locally by running ./runtest.sh.
  • In-line docstrings updated.
  • Documentation updated.

yanchengnv
yanchengnv approved these changes Aug 9, 2024
View reviewed changes
Copy link
Collaborator

@yanchengnv yanchengnv left a comment

Choose a reason for hiding this comment

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

This PR looks OK. But we are still scanning too much. Need another PR to address it.

YuanTingHsieh
YuanTingHsieh approved these changes Aug 9, 2024
View reviewed changes
Copy link
Collaborator

@YuanTingHsieh YuanTingHsieh left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM @holgerroth @ZiyueXu77 please check.

We have this change because of performance issues, we might need to modify other places like examples in MONAI toolkit as well

@chesterxgchen chesterxgchen changed the title app_opt scan changes name to path changes in configuration Aug 9, 2024
@YuanTingHsieh
Copy link
Collaborator

I will update xgboost related examples

@YuanTingHsieh
Copy link
Collaborator

/build

@YuanTingHsieh YuanTingHsieh merged commit a5bd071 into NVIDIA:main Aug 9, 2024
16 checks passed
@YuanTingHsieh YuanTingHsieh mentioned this pull request Aug 21, 2024
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@YuanTingHsieh YuanTingHsieh YuanTingHsieh approved these changes

@yanchengnv yanchengnv yanchengnv approved these changes

@chesterxgchen chesterxgchen Awaiting requested review from chesterxgchen

@IsaacYangSLA IsaacYangSLA Awaiting requested review from IsaacYangSLA

@holgerroth holgerroth Awaiting requested review from holgerroth

@nvkevlu nvkevlu Awaiting requested review from nvkevlu

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

Successfully merging this pull request may close these issues.
3 participants
@yhwen @YuanTingHsieh @yanchengnv