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

[oneDPL][RNG] Changing type of internal parameter of discard_block_engine #651

Merged
merged 2 commits into from Oct 24, 2022
Merged

[oneDPL][RNG] Changing type of internal parameter of discard_block_engine #651

merged 2 commits into from Oct 24, 2022

Conversation

egrabovskaya
Copy link
Contributor

@egrabovskaya egrabovskaya commented Oct 13, 2022 •
edited

Changing type of internal parameter from int to size_t.
Note: changes break ABI backward compatibility.

@paveldyakov
Copy link
Contributor

The failures in testing not related to PR:
1/1 Test #14: sycl_iterator.pass ...............Exit code 0xc0000409
***Exception: 0.71 sec
Cannot load TBB from neither Windows registry key nor CPU runtime configuration file (cl.cfg / cl.fpga_emu.cfg). Error: Windows error code: 126.
You can ask your administrator to configure TBB library location in the configuration file.

paveldyakov
paveldyakov previously approved these changes Oct 13, 2022
View reviewed changes
aelizaro
aelizaro previously approved these changes Oct 13, 2022
View reviewed changes
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.

You can make PR name shorter and move a note about ABI changes to description

@egrabovskaya egrabovskaya changed the title [oneDPL][RNG] Changing type of internal parameter of discard_block_engine from int to size_t. ABI breaking changes [oneDPL][RNG] Changing type of internal parameter of discard_block_engine Oct 13, 2022
rarutyun
rarutyun reviewed Oct 13, 2022
View reviewed changes
include/oneapi/dpl/internal/random_impl/discard_block_engine.h Outdated
@@ -205,7 +205,7 @@ class discard_block_engine
}

_Engine engine_;
int n_ = 0;
size_t n_ = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be ::std::size_t (actually, in every place of use) and it has missing include (e.g. cstddef). I am wondering how tests are passed. Seems like you don't include API headers first (before standard headers) or have some kind of umbrella header. Anyway, this header (and, probably, others) is not self-contained.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"std" namespace for size_t and missing includes are added

@egrabovskaya
Copy link
Contributor Author

We have two failed steps:
https://github.com/oneapi-src/oneDPL/actions/runs/3267430013/jobs/5372599082
https://github.com/oneapi-src/oneDPL/actions/runs/3267430013/jobs/5372599367

I suppose it's because the specified c++ version of the std compiler flag wasn't changed after this commit:
5a5b953
According to this, the failures are not related to the PR

@egrabovskaya egrabovskaya merged commit 47d4fcc into main Oct 24, 2022
@egrabovskaya egrabovskaya deleted the dev/egrabovs/fix_of_discard_block branch October 24, 2022 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rarutyun rarutyun rarutyun approved these changes

@aelizaro aelizaro aelizaro approved these changes

@paveldyakov paveldyakov paveldyakov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@egrabovskaya @paveldyakov @rarutyun @aelizaro