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

Log UMAP arrays at trace verbosity level. #6274

Merged
merged 2 commits into from
Feb 12, 2025

Conversation

csadorf
Copy link
Contributor

@csadorf csadorf commented Jan 29, 2025

Reduce the UMAP logging verbosity. Avoids printing potentially large arrays.

Copy link

copy-pr-bot bot commented Jan 29, 2025

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@csadorf csadorf force-pushed the fix/decrease-umap-logging-verbosity branch from 6cccf26 to d7efec6 Compare January 29, 2025 15:28
@csadorf
Copy link
Contributor Author

csadorf commented Jan 29, 2025

Looks like I might have missed on statement, I still get one large print. Will check and fixup. Done.

@csadorf csadorf added bug Something isn't working non-breaking Non-breaking change labels Jan 29, 2025
@csadorf csadorf self-assigned this Jan 29, 2025
@csadorf csadorf marked this pull request as ready for review January 29, 2025 15:50
@csadorf csadorf requested a review from a team as a code owner January 29, 2025 15:50
@csadorf csadorf requested review from vyasr and tfeher January 29, 2025 15:50
if (ML::default_logger().should_log(ML::level_enum::debug)) {
CUML_LOG_DEBUG("Compute Membership Strength");
if (ML::default_logger().should_log(ML::level_enum::trace)) {
CUML_LOG_TRACE("Compute Membership Strength");
Copy link
Member

Choose a reason for hiding this comment

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

I think we should keep these at DEBUG level but put any actual arrays at trace level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in c6b6e36 .

@csadorf csadorf requested a review from cjnolet January 29, 2025 16:00
@csadorf csadorf changed the base branch from branch-25.02 to branch-25.04 February 10, 2025 22:35
@csadorf
Copy link
Contributor Author

csadorf commented Feb 10, 2025

/merge

@dantegd dantegd changed the base branch from branch-25.04 to branch-25.02 February 11, 2025 21:44
@dantegd dantegd requested review from a team as code owners February 11, 2025 21:44
@csadorf csadorf force-pushed the fix/decrease-umap-logging-verbosity branch from d7af773 to 16f79b3 Compare February 11, 2025 21:54
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.07%. Comparing base (f60b5f0) to head (16f79b3).

Additional details and impacted files
@@               Coverage Diff                @@
##           branch-25.02    #6274      +/-   ##
================================================
- Coverage         68.24%   67.07%   -1.18%     
================================================
  Files               202      202              
  Lines             13079    13069      -10     
================================================
- Hits               8926     8766     -160     
- Misses             4153     4303     +150     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@csadorf
Copy link
Contributor Author

csadorf commented Feb 12, 2025

@dantegd I think this PR might need an admin merge.

@raydouglass raydouglass merged commit 448b624 into branch-25.02 Feb 12, 2025
69 of 71 checks passed
@jakirkham jakirkham deleted the fix/decrease-umap-logging-verbosity branch February 13, 2025 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CUDA/C++ non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants