Skip to content

Conversation

@jle-quel
Copy link
Contributor

@jle-quel jle-quel commented Sep 4, 2023

Description

This PR updates the rocRAND backend's distribution functions which used to be unimplemented.

The following distribution functions have been implemented for the philo and mrg generators, for both API (Buffer and USM):

  • gaussian<float, oneapi::mkl::rng::gaussian_method::icdf>
  • gaussian<double, oneapi::mkl::rng::gaussian_method::icdf>
  • lognormal<float, oneapi::mkl::rng::lognormal_method::icdf>
  • lognormal<double, oneapi::mkl::rng::lognormal_method::icdf>
  • poisson<std::int32_t, poisson_method::gaussian_icdf_based>
  • poisson<std::uint32_t, poisson_method::gaussian_icdf_based>

While updating these functions, a race condition has also been fixed, which makes the gaussian distribution functions pass again. The reason for the race condition was due to the asynchronous host_task which launched asynchronouss rocRAND computation. The stream created for that computation was not synchronized.

Similar issues raised on cuBLAS: #175 (comment)

Furthermore, the copy constructor has also been implemented and a seed_ and an offset_ member has been introduced to the philo_impl and mrg_impl to be able to track and set the same seed, and also align the offset of the engines if the * other already generated values.

Finally, the constructor for philo and mrg, and the function skip_ahead which takes a std::initializer_list are still marked as unimplemented due to rocRAND not supporting std::initializer_list.
Also, the bernoulli distribution function is still marked as unimplemented due to rocRAND not supporting this distribution method.

Checklist

All Submissions

  • Do all unit tests pass locally? Attach a log.
  • Have you formatted the code using clang-format?

example_rng_uniform_usm.log.txt
test_main_rng_ct.log.txt
test_main_rng_rt.log.txt

Copy link
Contributor

@aelizaro aelizaro left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you very much @jle-quel!

@aelizaro aelizaro requested a review from mkrainiuk September 4, 2023 14:28
@aelizaro
Copy link
Contributor

@mmeterel, can you please take a look from a general perspective?

@aelizaro aelizaro requested a review from mmeterel September 11, 2023 09:11
Copy link
Contributor

@mmeterel mmeterel left a comment

Choose a reason for hiding this comment

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

@aelizaro Thanks for the PR.
It looks good overall. I have two comments:

  1. I don't see any updates in README regarding supported backends. It is missed?
  2. Did you file a JIRA to infra team to enable this backend in regular testing?

@aelizaro
Copy link
Contributor

@aelizaro Thanks for the PR. It looks good overall. I have two comments:

  1. I don't see any updates in README regarding supported backends. It is missed?
  2. Did you file a JIRA to infra team to enable this backend in regular testing?

Hi @mmeterel, thank you for the review! This is @jle-quel's PR which extends and fixes the existing rocRAND backend, so we already have 1 done and 2nd should already exist.

@jle-quel, thanks again! I can merge PR later today.

@aelizaro aelizaro merged commit 0d74ee4 into uxlfoundation:develop Sep 12, 2023
@jle-quel jle-quel deleted the jle-quel/update-rocrand-backend branch September 12, 2023 09:09
normallytangent pushed a commit to normallytangent/oneMKL that referenced this pull request Aug 6, 2024
* Fix typo in HIP_ERROR_FUNC macro

* Fix race condition in host_task's lambda

* Update unimplemented philo's function which can be now be supported by rocRAND

* Update unimplemented mrg's function which can be now be supported by rocRAND
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants