Skip to content

Add support of fp32 for sycl implementations #308

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
Merged
merged 2 commits into from
Nov 8, 2023
Merged

Add support of fp32 for sycl implementations #308

merged 2 commits into from
Nov 8, 2023

Conversation

@AlexanderKalistratov
Copy link
Collaborator

@AlexanderKalistratov AlexanderKalistratov commented Oct 26, 2023

Add support of fp32 for sycl implementations except pairwise distance.
pairwise distance added here #306

ZzEeKkAa
ZzEeKkAa approved these changes Oct 26, 2023
View reviewed changes
Copy link
Contributor

@ZzEeKkAa ZzEeKkAa left a comment •
edited
Loading

Choose a reason for hiding this comment

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

These are awesome changes! Thank you!

Could you please run pre-commit to your commit and push it again?

UPD: should we add sycl float32 run in workflow?

@AlexanderKalistratov
Copy link
Collaborator Author

AlexanderKalistratov commented Oct 27, 2023

@ZzEeKkAa
I've added job to run dpbench with fp32 precision. Please take a look if it ok.
Also dpnp crashed on pca workload with fp32 precision.
What should we do?

@ZzEeKkAa
Copy link
Contributor

ZzEeKkAa commented Oct 30, 2023 •
edited
Loading

@ZzEeKkAa I've added job to run dpbench with fp32 precision. Please take a look if it ok. Also dpnp crashed on pca workload with fp32 precision. What should we do?

Let's run fp32 only for sycl implementation

UPD: it is more likely issue with CI system, not dpnp. Please see later comment

ZzEeKkAa
ZzEeKkAa reviewed Oct 30, 2023
View reviewed changes
adarshyoga
adarshyoga reviewed Nov 1, 2023
View reviewed changes
...marks/black_scholes/black_scholes_sycl_native_ext/black_scholes_sycl/_black_scholes_sycl.cpp
Comment on lines +67 to +81
if (typenum == UAR_FLOAT) {
black_scholes_impl<float>(Queue, nopt, price.get_data<float>(),
strike.get_data<float>(), t.get_data<float>(),
rate, volatility, call.get_data<float>(),
put.get_data<float>());
}
else if (typenum == UAR_DOUBLE) {
black_scholes_impl<double>(
Queue, nopt, price.get_data<double>(), strike.get_data<double>(),
t.get_data<double>(), rate, volatility, call.get_data<double>(),
put.get_data<double>());
}
else {
throw std::runtime_error(
"Expected a double or single precision FP array.");
Copy link
Contributor

@adarshyoga adarshyoga Nov 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It should be possible to directly bind functions with template parameters. This might be a neater solution. @oleksandr-pavlyk your thoughts?

@AlexanderKalistratov AlexanderKalistratov force-pushed the fp32_support_for_sycl_impl branch 2 times, most recently from 58c67bc to 5788a12 Compare November 3, 2023 15:26
@ZzEeKkAa ZzEeKkAa force-pushed the fp32_support_for_sycl_impl branch 3 times, most recently from b0fb043 to 75c62db Compare November 6, 2023 00:37
@ZzEeKkAa
Copy link
Contributor

ZzEeKkAa commented Nov 6, 2023 •
edited
Loading

@AlexanderKalistratov could you please clean commit history? e.g: you are modifying .github folder in separate commits. I did rebase to check if all tests pass (there was fix in main for windows compilation)

ZzEeKkAa
ZzEeKkAa reviewed Nov 8, 2023
View reviewed changes
@ZzEeKkAa ZzEeKkAa force-pushed the fp32_support_for_sycl_impl branch from 90df638 to b6d1bac Compare November 8, 2023 19:40
@ZzEeKkAa ZzEeKkAa mentioned this pull request Nov 8, 2023
Closed
@ZzEeKkAa ZzEeKkAa merged commit f5e452b into IntelPython:main Nov 8, 2023
@ZzEeKkAa
Copy link
Contributor

ZzEeKkAa commented Nov 8, 2023

Rambo numba_mlir fix: numba/numba-mlir#331

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@ZzEeKkAa ZzEeKkAa ZzEeKkAa approved these changes

@adarshyoga adarshyoga adarshyoga left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@AlexanderKalistratov @ZzEeKkAa @adarshyoga