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

Disable cost saving functionality #1218

Merged
merged 2 commits into from
Jul 23, 2024

Conversation

amahussein
Copy link
Collaborator

@amahussein amahussein commented Jul 23, 2024

Signed-off-by: Ahmed Hussein (amahussein) [email protected]

Fixes #1217

  • this change adds a configuration flag that force disabling the cost savings
  • even when a cluster or platform is part of teh argument, the Q tool output won't include any data related to costs.
  • this is temp solution to disable the cost-savings. Later, there will be other issues to cleanup the dead-code

The full list of tasks is tracked in #1221

Signed-off-by: Ahmed Hussein (amahussein) <[email protected]>

Fixes NVIDIA#1217

- this change adds aconfiguration glag that force disabling the cost savings
- even when a cluster or platform is part of teh argument, the Q tool
  output won't include any data related to costs.
- this is temp solution to disable the cost-savings. Later, there will
  be other issues to cleanup the dead-code
@amahussein amahussein added feature request New feature or request user_tools Scope the wrapper module running CSP, QualX, and reports (python) labels Jul 23, 2024
@amahussein amahussein self-assigned this Jul 23, 2024
Signed-off-by: Ahmed Hussein (amahussein) <[email protected]>
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. I have few questions:

  1. What are the plans for the argument --filter_apps savings? It would currently throw an exception for the following CMD:
spark_rapids qualification --platform databricks-aws  --eventlogs $EVENTLOG_PATH  --tools_jar $SPARK_RAPIDS_TOOLS_JAR --filter_apps savings --cluster ~/Work/cluster/databricks-aws/databricks-aws-cpu.json

2 Do we plan to remove 'SAVINGS' word from the console output when we clean up the csv files columns?

    - Summarized savings and speedups CSV report: /Users/psarthi/Work/tools-run/qual_20240723194339_11E13C20/qualification_summary.csv
    - Full savings and speedups CSV report: /Users/psarthi/Work/tools-run/qual_20240723194339_11E13C20/qualification_summary_full.csv

@amahussein
Copy link
Collaborator Author

Thanks @amahussein. LGTME. I have few questions:

1. What are the plans for the argument `--filter_apps savings`? It would currently throw an exception for the following CMD:
spark_rapids qualification --platform databricks-aws  --eventlogs $EVENTLOG_PATH  --tools_jar $SPARK_RAPIDS_TOOLS_JAR --filter_apps savings --cluster ~/Work/cluster/databricks-aws/databricks-aws-cpu.json

2 Do we plan to remove 'SAVINGS' word from the console output when we clean up the csv files columns?

    - Summarized savings and speedups CSV report: /Users/psarthi/Work/tools-run/qual_20240723194339_11E13C20/qualification_summary.csv
    - Full savings and speedups CSV report: /Users/psarthi/Work/tools-run/qual_20240723194339_11E13C20/qualification_summary_full.csv

Yes, we need to remove arguments related to the cost-savings, and do all the other logistics.
However, this is going to be some sort of long process that includes cleaning up the code and refactoring.

The purpose of this PR is an immediate block to cancel Cost-savings.

  • Since the cost savings are automatically generated when the cluster-information is provided,
  • cost-savings does not match what the Tuning path is doing.
  • The tuning generates output per-app, while cost savings generates output per-cluster.

@amahussein amahussein requested a review from parthosa July 23, 2024 20:19
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. RIP cost savings.

@amahussein amahussein merged commit 0b74b6c into NVIDIA:dev Jul 23, 2024
14 checks passed
@amahussein amahussein deleted the spark-rapids-tools-1199-1217 branch July 23, 2024 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request user_tools Scope the wrapper module running CSP, QualX, and reports (python)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] Disable cost savings functionality in qualification CLI
3 participants