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

Changing autotuner memory error to warning in comments #1418

Merged
merged 3 commits into from
Nov 14, 2024

Conversation

mattahrens
Copy link
Collaborator

Fixes #1314

@mattahrens mattahrens added the core_tools Scope the core module (scala) label Nov 12, 2024
@mattahrens mattahrens self-assigned this Nov 12, 2024
parthosa
parthosa previously approved these changes Nov 13, 2024
Copy link
Collaborator

@parthosa parthosa left a comment

Choose a reason for hiding this comment

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

Thanks @mattahrens for making this change. I was also thinking we should change this to warn as it could cause issue with internal testing team as well.

@parthosa parthosa added the bug Something isn't working label Nov 13, 2024
amahussein
amahussein previously approved these changes Nov 13, 2024
val msg = "This node/worker configuration is not ideal for using the Spark Rapids " +
"Accelerator because it doesn't have enough memory for the executors. " +
val msg = "This node/worker configuration is not ideal for using the Spark Rapids \n" +
"Accelerator because it doesn't have enough memory for the executors. \n" +
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: trailing space before the \n

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated in latest commit

// in the future it would be nice to enhance the error message with a recommendation of size
val msg = "This node/worker configuration is not ideal for using the Spark Rapids " +
"Accelerator because it doesn't have enough memory for the executors. " +
val msg = "This node/worker configuration is not ideal for using the Spark Rapids \n" +
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: trailing space before the \n

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated in latest commit

@mattahrens mattahrens dismissed stale reviews from amahussein and parthosa via eebb002 November 13, 2024 21:48
@mattahrens mattahrens merged commit acec193 into NVIDIA:dev Nov 14, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core_tools Scope the core module (scala)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Change autotuner memory to small error to a warning
3 participants