-
Notifications
You must be signed in to change notification settings - Fork 242
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
Added in fallback tests #174
Conversation
ok to test |
build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks great , just a couple minor comments for your perusal.
incompat: Boolean = false, | ||
execsAllowedNonGpu: Seq[String] = Seq.empty, | ||
sortBeforeRepart: Boolean = false) | ||
(fun: DataFrame => DataFrame): Unit = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might need to fix the scalastyle per
For method declarations, use 4 space indentation for their parameters and put each in each line when the parameters don't fit in two lines. Return types can be either on the same line as the last parameter, or start a new line with 2 space indent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The scala style check passes for this. Not sure where the issue is?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry , I am looking at https://github.com/databricks/scala-style-guide#indent and I see the 4 indents and newline pattern used in Spark code base per this guideline as well.
def runOnCpuAndGpuWithCapture( | ||
df: SparkSession => DataFrame, | ||
fun: DataFrame => DataFrame, | ||
conf: SparkConf = new SparkConf(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit : indent seems off to me
build |
build |
Approved pending green CI. |
Signed-off-by: spark-rapids automation <[email protected]>
This closes #167
We use Spark's metrics callback to capture the plan after the GPU session has run so we can verify that it did fall back as expected. With this I found a test that was not falling back any more, because the underlying problem was fixed that it was testing for.