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

Remove legacy SpeedupFactor from core output files #1318

Merged

Conversation

amahussein
Copy link
Collaborator

Signed-off-by: Ahmed Hussein ahussein@nvidia.com

Contributes to #1199

Remove legacy speedup columns from core tools output files. This does not address removing fields from AppSummaryInfo which requires extensive code cleanup

  • rapids_4_spark_qualification_output.csv
    • Estimated GPU Speedup
    • Task Speedup Factor
    • Recommendation
  • rapids_4_spark_qualification_output_execs.csv
    • Task Speedup Factor
  • rapids_4_spark_qualification_output_stages.csv
    • Average Speedup Factor
  • rapids_4_spark_qualification_output_persql.csv
    • Recommendation
    • Estimated GPU Speedup

Signed-off-by: Ahmed Hussein <ahussein@nvidia.com>

Contributes to NVIDIA#1199

Remove legacy speedup columns from core tools output files. This does
not address removing fields from AppSummaryInfo which requires extensive
code cleanup

- `rapids_4_spark_qualification_output.csv`
  - `Estimated GPU Speedup`
  - `Task Speedup Factor`
  - `Recommendation`
- `rapids_4_spark_qualification_output_execs.csv`
  - `Task Speedup Factor`
- `rapids_4_spark_qualification_output_stages.csv`
  - `Average Speedup Factor`
- `rapids_4_spark_qualification_output_persql.csv`
  - `Recommendation`
  - `Estimated GPU Speedup`
@amahussein amahussein added core_tools Scope the core module (scala) affect-output A change that modifies the output (add/remove/rename files, add/remove/rename columns) labels Aug 28, 2024
@amahussein amahussein self-assigned this Aug 28, 2024
@amahussein amahussein marked this pull request as draft August 28, 2024 21:31
@tgravescs
Copy link
Collaborator

this seems to be in draft what is left here?

@amahussein
Copy link
Collaborator Author

this seems to be in draft what is left here?

Making some fixes on the Python Qualx side that should not affect the Scala changes.

Signed-off-by: Lee Yang <leewyang@gmail.com>
@leewyang leewyang marked this pull request as ready for review August 29, 2024 17:05
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 @amahussein. LGTME.

From our offline discussion:

  • In python, we replace the legacy col ESTIMATED_GPU_DURATION from scala by appDuration_pred from qualx and recalculate ESTIMATED_GPU_TIMESAVED.
  • We should remove these legacy GPU cols as well. This could be a follow PR.

@amahussein
Copy link
Collaborator Author

Thanks @leewyang for adding the patch removing unnecessary reads from the core's output.
@tgravescs Are you fine with the changes in this PR?

@cindyyuanjiang cindyyuanjiang self-requested a review August 29, 2024 21:57
Copy link
Collaborator

@cindyyuanjiang cindyyuanjiang left a comment

Choose a reason for hiding this comment

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

Thanks Ahmed! There may be some merge conflicts with #1320 depending on which PR goes in first.

@amahussein amahussein merged commit d9b7a52 into NVIDIA:dev Aug 30, 2024
14 checks passed
@amahussein amahussein deleted the rapids-tools-1199-remove-speedupfactor branch August 30, 2024 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affect-output A change that modifies the output (add/remove/rename files, add/remove/rename columns) core_tools Scope the core module (scala)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants