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

[FEA] Add user qualification tool options for specifying pricing discounts for CPU or GPU cluster, or both #583

Merged
merged 4 commits into from
Sep 27, 2023

Conversation

cindyyuanjiang
Copy link
Collaborator

@cindyyuanjiang cindyyuanjiang commented Sep 22, 2023

Fixes #476

We add three new user qualification tool options (for both spark_rapids_user_tools and ascli CLI)

  • --cpu_discount: the specified percentage discount for the CPU cluster cost to be reduced by (e.g. 30 for 30% discount)
  • --gpu_discount: the specified percentage discount for the GPU cluster cost to be reduced by
  • --global_discount: the specified percentage discount for both cluster costs to be reduced by. Note that if --global_discount is specified, neither --cpu_discount nor --gpu_discount should be specified, otherwise the tool will raise an exception

Follow up work:
We need to update documentation, issue tracked here: #584

… spark_rapids_user_tools

Signed-off-by: cindyyuanjiang <[email protected]>
Signed-off-by: cindyyuanjiang <[email protected]>
Signed-off-by: cindyyuanjiang <[email protected]>
@cindyyuanjiang cindyyuanjiang self-assigned this Sep 22, 2023
@cindyyuanjiang cindyyuanjiang added feature request New feature or request user_tools Scope the wrapper module running CSP, QualX, and reports (python) labels Sep 22, 2023
@cindyyuanjiang
Copy link
Collaborator Author

cindyyuanjiang commented Sep 22, 2023

We tested the PR change with the following script:

export EVENT_LOGS=... # a local event log file
export PLATFORM=... # the csp platform we want to test
export CPU_CLUSTER_PROPS=... # the cpu cluster properties file for the relative platform

########################################
# running each of the CSP platforms
########################################

# run the following commands and confirm the discount calculations are correct
spark_rapids_user_tools $PLATFORM qualification \
  --eventlogs $EVENT_LOGS \
  --cpu_cluster $CPU_CLUSTER_PROPS

spark_rapids_user_tools $PLATFORM qualification \
  --eventlogs $EVENT_LOGS \
  --cpu_cluster $CPU_CLUSTER_PROPS \
  --cpu_discount 30

spark_rapids_user_tools $PLATFORM qualification \
  --eventlogs $EVENT_LOGS \
  --cpu_cluster $CPU_CLUSTER_PROPS \
  --cpu_discount 30 \
  --gpu_discount 30

ascli qualification \
  --platform $PLATFORM \
  --eventlogs $EVENT_LOGS \
  --cluster $CPU_CLUSTER_PROPS

ascli qualification \
  --platform $PLATFORM \
  --eventlogs $EVENT_LOGS \
  --cluster $CPU_CLUSTER_PROPS \
  --gpu_discount 30

ascli qualification \
  --platform $PLATFORM \
  --eventlogs $EVENT_LOGS \
  --cluster $CPU_CLUSTER_PROPS \
  --cpu_discount 30 \
  --gpu_discount 30

########################################
# failures
########################################

# running a command when global_discount is specified with cpu_discount, gpu_discount, and both

# running a command with non-integer input for cpu_discount, gpu_discount, and global_discount

# running a command with int input for cpu_discount, gpu_discount, and global_discount that is out of range [0, 100]

nartal1
nartal1 previously approved these changes Sep 26, 2023
Copy link
Collaborator

@nartal1 nartal1 left a comment

Choose a reason for hiding this comment

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

Thanks @cindyyuanjiang! Just a nit to remove redundancy and better readability. Otherwise, LGTM.

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 @cindyyuanjiang. Added a few minor comments.

user_tools/pyproject.toml Outdated Show resolved Hide resolved
Signed-off-by: cindyyuanjiang <[email protected]>
@cindyyuanjiang cindyyuanjiang merged commit 0afe2e8 into NVIDIA:dev Sep 27, 2023
8 checks passed
@cindyyuanjiang cindyyuanjiang deleted the set-price-discount branch September 27, 2023 01:09
@cindyyuanjiang cindyyuanjiang added the new-cli scope out future new-cli work for the next few months label Sep 28, 2023
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 new-cli scope out future new-cli work for the next few months user_tools Scope the wrapper module running CSP, QualX, and reports (python)
Projects
None yet
3 participants