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

Unified platform handling and fetching of operator score files #661

Merged
merged 7 commits into from
Nov 20, 2023

Conversation

parthosa
Copy link
Collaborator

@parthosa parthosa commented Nov 15, 2023

Fixes #616.

This PR fixes the below issues related to platform argument:

  1. Correct platform handling
    • Original scope to handle case when the platform provided is incorrect in Qualification tools.
  2. Unified platform handling
  3. Improve fetching of operator score files
    • Revise fetching of operator score files based on platform name. It is currently hardcoded for specific platforms and GPU types.

Major Changes:

  1. Platform.scala:
    • Moved Platform.scala outside the profiler package for shared use.
    • Added PlatformNames to store platform strings as constants.
    • Added PlatformFactory to create Platform instances based on platform string values.
      • Handles cases where the platform is incorrect (original issue). Addresses #1
      • Both Profiler and Qualification tool use createInstance() to get platform instance. Addresses #2
  2. PluginTypeChecker.scala:
    • Use method getOperatorScoreFile() defined in Platform.scala for dynamic filename generation of operator score files. Addresses #3

Follow Up:

  • Currently there are many instances (unit tests, default value) where platform is still passed as a string value. I plan to submit a follow up PR to replace all of them with the constants defined in PlatformNames class.

@parthosa parthosa added the core_tools Scope the core module (scala) label Nov 15, 2023
@parthosa parthosa self-assigned this Nov 15, 2023
Signed-off-by: Partho Sarthi <[email protected]>
@parthosa parthosa changed the title Handle incorrect platform in core tools Improve handling of string based platform types and operator score files Nov 16, 2023
@parthosa parthosa changed the title Improve handling of string based platform types and operator score files Unified platform handling and enhanced operator score files Nov 16, 2023
@parthosa parthosa marked this pull request as ready for review November 16, 2023 22:12
Signed-off-by: Partho Sarthi <[email protected]>
@parthosa parthosa requested a review from tgravescs November 17, 2023 00:16
Signed-off-by: Partho Sarthi <[email protected]>
@kuhushukla
Copy link
Collaborator

LGTM overall. Would it be right to say that the operator score changes here allow for filenames that tell us which platform they are for and not necessarily the score aspect of it? Thank you.

@parthosa parthosa changed the title Unified platform handling and enhanced operator score files Unified platform handling and naming of operator score files Nov 20, 2023
@parthosa parthosa changed the title Unified platform handling and naming of operator score files Unified platform handling and fetching of operator score files Nov 20, 2023
@parthosa parthosa merged commit e612d79 into NVIDIA:dev Nov 20, 2023
8 checks passed
@parthosa parthosa deleted the spark-rapids-tools-616 branch November 20, 2023 18:00
@parthosa parthosa restored the spark-rapids-tools-616 branch November 20, 2023 18:44
@parthosa parthosa deleted the spark-rapids-tools-616 branch November 20, 2023 18:49
@parthosa parthosa added the bug Something isn't working label Dec 18, 2023
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] Qualification tool: Invalid --platform doesn't error
4 participants