Skip to content

Conversation

samir-nasibli
Copy link

@samir-nasibli samir-nasibli commented Jan 15, 2021

Description

  • added stream for classic MKL call
  • update radnom.beta backend func, when GPU selector

TODO

  • backend tests -> TODO pytest_cpp will be added -> other PR
  • update example5.cpp -> other PR
    • rename backend_sycl -> other PR

@samir-nasibli samir-nasibli added the in progress Please do not merge. Work is in progress. label Jan 15, 2021
@samir-nasibli samir-nasibli removed the in progress Please do not merge. Work is in progress. label Jan 15, 2021
@samir-nasibli
Copy link
Author

@shssf this PR is ready for review/merge.
We just need rename backend_sycl.
Also we need to gtests for backend, that could be done on other PR.

@samir-nasibli samir-nasibli requested a review from shssf January 15, 2021 19:29
@shssf
Copy link
Contributor

shssf commented Jan 16, 2021

@samir-nasibli It looks like we need to rename dpnp_srand_c() to dpnp_rng_srand_c(). I didn't do this in this PR to be clear.

Also, it is not quite clear why you removed fmap[DPNPFuncName::DPNP_FN_RNG_BETA][eft_FLT][eft_FLT]. Is it a typo?

please note, I didn't check this on GPU.

@samir-nasibli
Copy link
Author

@samir-nasibli It looks like we need to rename dpnp_srand_c() to dpnp_rng_srand_c(). I didn't do this in this PR to be clear.

Yes, we can do in on other PR.

@samir-nasibli
Copy link
Author

Also, it is not quite clear why you removed fmap[DPNPFuncName::DPNP_FN_RNG_BETA][eft_FLT][eft_FLT]. Is it a typo?

No, this is not typo. We do not use fmap[DPNPFuncName::DPNP_FN_RNG_BETA][eft_FLT][eft_FLT] at all.

@samir-nasibli
Copy link
Author

please note, I didn't check this on GPU.

We can wait with merge, it it is needed. We can do it after adding gtest tests for backend.

Comment on lines +39 to +70
/**
* Use get/set functions to access/modify this variable
*/
static VSLStreamStatePtr rng_stream = nullptr;

static void set_rng_stream(size_t seed = 1)
{
if (rng_stream)
{
vslDeleteStream(&rng_stream);
rng_stream = nullptr;
}

vslNewStream(&rng_stream, VSL_BRNG_MT19937, seed);
}

static VSLStreamStatePtr get_rng_stream()
{
if (!rng_stream)
{
set_rng_stream();
}

return rng_stream;
}

void dpnp_srand_c(size_t seed)
{
backend_sycl::backend_sycl_rng_engine_init(seed);
set_rng_stream(seed);
}

Copy link
Author

Choose a reason for hiding this comment

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

I don't think that this is simpler.
I vote to save it as it was and revert you changes, except throw exception.

@shssf shssf merged commit e02bd94 into master Jan 16, 2021
@shssf shssf deleted the samir-nasibli/enh/beta_backend_fallback branch January 16, 2021 17:24
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.

2 participants