SEPURI-SAI-KRISHNA commented on PR #58149: URL: https://github.com/apache/spark/pull/58149#issuecomment-5586852745
@uros-b thanks, both are right. Fixed. On the test comment: `vectorInfNorm` is a plain scalar loop, so there is no unrolled section on that path, and the comment was wrong. Only `vectorL1Norm` and `vectorL2Norm` use the 8-element unrolling. I reworded it to describe what the case actually covers, a NaN element in a longer vector. The same wrong wording appeared in the "How was this patch tested?" section, so I corrected that too. Worth noting that this PR changes only `vectorInfNorm`, so the unrolled paths are not touched by the fix at all. On the description table: you are right, the pre-fix value is `[NaN, 1.0]`. The old infinity norm skipped the NaN, because `NaN > maxAbs` is false, and returned `5.0`, so normalizing gave `[NaN/5, 5/5]`. I have corrected that row. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
