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.

Sign up for GitHub

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

Jump to bottom

[FEA] Another look at infinity checking on casting string to float #4152

Closed
revans2 opened this issue Nov 18, 2021 · 2 comments
Closed

[FEA] Another look at infinity checking on casting string to float #4152

revans2 opened this issue Nov 18, 2021 · 2 comments
Assignees
@HaoYang670
Labels
cudf_dependency An issue or PR with this label depends on a new feature in cudf good first issue Good for newcomers performance A performance related task/issue task Work required that improves the product but is not user facing

Comments

@revans2
Copy link
Collaborator

revans2 commented Nov 18, 2021

rapidsai/cudf#9694 is supposed to make checking for Inf case insensitive. This should let us totally remove the normalization for infinity using a regular expression when casting from strings to floats.

This might not matter if we make our own string to float casting at some point in the future.
@revans2 revans2 added ? - Needs Triage Need team to review and classify performance A performance related task/issue task Work required that improves the product but is not user facing cudf_dependency An issue or PR with this label depends on a new feature in cudf labels Nov 18, 2021
@sameerz sameerz added good first issue Good for newcomers and removed ? - Needs Triage Need team to review and classify labels Nov 23, 2021
@HaoYang670 HaoYang670 self-assigned this Dec 17, 2021
@HaoYang670
Copy link
Collaborator

Sorry, I read the code of castStringToFloats and sanitizeStringToFloat but don't find where "infinity" normalization is.

@jlowe
Copy link
Member

jlowe commented Dec 21, 2021

This has already been handled by #4259. See https://github.com/NVIDIA/spark-rapids/pull/4259/files#diff-e981882f5ee2f922528de849ae5397dd30e5bfe9dd5fdbe3421de4733a0eae1aL206-L218.

@jlowe jlowe closed this as completed Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@HaoYang670 HaoYang670

Labels
cudf_dependency An issue or PR with this label depends on a new feature in cudf good first issue Good for newcomers performance A performance related task/issue task Work required that improves the product but is not user facing
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jlowe @revans2 @sameerz @HaoYang670