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

QualificationTool. Add speedup information to AppSummaryInfo #5454

Merged
merged 2 commits into from
May 11, 2022

Conversation

amahussein
Copy link
Collaborator

Signed-off-by: Ahmed Hussein (amahussein) a@ahussein.me

fixes #5449

Splitting the scope of #5176 and the pending PR #5407

  • The changes are small. However, the total files affected by the change increased because of the CSV file expectations.

  • Adds the following fields to QualificationSummaryInfo

        longestSqlDuration: Long,
        nonSqlTaskDurationAndOverhead: Long,
        estimatedDuration: Double,
        unsupportedDuration: Long,
        speedupDuration: Long,
        speedupFactor: Double,
        totalSpeedup: Double,
        speedupBucket: String
    
  • Moved gpuMode and jobIdToInfo to AppBase because it is common between Qualification and Profiling tool

  • Moved some common code to EventProcessorBase

  • Added the new fileds to the CSV schema of QualificationSuite and updated all the expectations. Note that those values are generated as a stub and not accurate.

  • Updated QualOutputWriter to dump the new fields and the Recommendation to the summary

  • Added UI flag to the arguments. It is false by default and enabling it has no effect until the static code generation gets merged.

Signed-off-by: Ahmed Hussein (amahussein) <a@ahussein.me>
@amahussein amahussein added this to the May 2 - May 20 milestone May 10, 2022
@amahussein amahussein requested review from tgravescs and nartal1 May 10, 2022 21:15
@amahussein amahussein self-assigned this May 10, 2022
@@ -34,7 +34,7 @@ import org.apache.spark.sql.rapids.tool.qualification._
class Qualification(outputDir: String, numRows: Int, hadoopConf: Configuration,
timeout: Option[Long], nThreads: Int, order: String,
pluginTypeChecker: PluginTypeChecker, readScorePercent: Int,
reportReadSchema: Boolean, printStdout: Boolean) extends Logging {
reportReadSchema: Boolean, printStdout: Boolean, uiEnabled: Boolean = false) extends Logging {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would remove the uiEnabled option since its not included here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree. uiEnabled option and it's related code can be added in a different PR where we integrate the UI.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed the new argument.

Signed-off-by: Ahmed Hussein (amahussein) <a@ahussein.me>
@tgravescs
Copy link
Collaborator

build

@tgravescs tgravescs merged commit af14838 into NVIDIA:branch-22.06 May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] QualificationTool. Add speedup information to AppSummaryInfo
3 participants