Skip to content

Add missing includes in random implementation; fix issue in partial specialization#2620

Open
ferdymercury wants to merge 3 commits intouxlfoundation:mainfrom
ferdymercury:randomimplfixes
Open

Add missing includes in random implementation; fix issue in partial specialization#2620
ferdymercury wants to merge 3 commits intouxlfoundation:mainfrom
ferdymercury:randomimplfixes

Conversation

@ferdymercury
Copy link

@ferdymercury ferdymercury commented Mar 16, 2026

I would like to use the headers inside random_impl as a standalone header-only library compiled with SYCL support.

In other words, I have a file main.cpp that includes normaldistribution.h which I compile with -I/path/to/random_impl and the -fsycl flags.

Copy link
Contributor

@danhoeflinger danhoeflinger left a comment

Choose a reason for hiding this comment

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

Can you provide a small description of what specifically you are fixing here in the description?
I think these are all positive changes, but it would be useful.

With a description and change to using sycl_defs, I think this looks good.

@ferdymercury
Copy link
Author

I just updated the description.

it's supposed to happen already in sycl_defs within the collection header oneapi/dpl/random
@danhoeflinger danhoeflinger changed the title include relevant headers, do not rely on inclusion order, only allow transitive sycl inclusion for randomcommon Add missing includes in random implementation; fix issue in partial specialization Mar 20, 2026
@danhoeflinger
Copy link
Contributor

@ferdymercury this LGTM, but we are in the middle of trying to get a release packaged up so it will need to wait a little bit to be merged. (I hope ~1 week). Thanks for the contribution!

@ferdymercury
Copy link
Author

@ferdymercury this LGTM, but we are in the middle of trying to get a release packaged up so it will need to wait a little bit to be merged. (I hope ~1 week). Thanks for the contribution!

Thanks. No hurries at all from my side

Copy link
Contributor

@danhoeflinger danhoeflinger left a comment

Choose a reason for hiding this comment

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

Just putting a hold on this until we get through the release window here. No changes are needed.

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