Skip to content

Add oneDPL Public CI state and requirements. #222

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.

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

Closed
wants to merge 17 commits into from

Conversation

timmiesmith
Copy link
Collaborator

@timmiesmith timmiesmith commented Apr 18, 2025

Initial capture of the current state of oneDPL Public CI and what would be needed to fully test community contributions.

Fixes #208

@timmiesmith timmiesmith changed the title Draft of oneDPL Public CI state and requirements. Add oneDPL Public CI state and requirements. Apr 21, 2025
Copy link

@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.

This LGTM. I think it should be everything

* Add oneMKL info to project-ci-documentation.md

* Correct some typos

* Update project-ci-documentation.md

* Update project-ci-documentation.md
| gmake | clang++ compiler | |
| ninja | g++ | |
| Visual Studio 2019 | | |
| Visual Studio 2022 | | |

Choose a reason for hiding this comment

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

I think we should also mention the supported GPU driver and their versions. It is a yet another tool oneDPL depends on.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think this is too much detail at this point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The question of how much detail is needed in this document was discussed in the UXL Open Source Working Group meeting. The feedback received is that any details on configurations teams can provide is useful, so I will add the details suggested.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

* CMake
* glibc
* ...
| Windows | Linux | MacOS (Arm CPU testing) |

Choose a reason for hiding this comment

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

I think it would be more readable to have "Software" or "Tool" as a first column. The cells may contain for example: minimal version number, "any" or "-" (not needed).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

version information is too much detail in my opinion.

Choose a reason for hiding this comment

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

Different product on one line still bothers me (such as dpc++ compiler and clang++ compiler). I suggest making a bullet list.

You can ignore this comment though - it is a matter of taste.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've added the Software column.

| Dan Hoeflinger @danhoeflinger |
| Dmitriy Sobolev @dmitriy-sobolev |
| Timmie Smith @timmiesmith |

*Existing public CI*
Copy link

@dmitriy-sobolev dmitriy-sobolev Apr 22, 2025

Choose a reason for hiding this comment

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

I think it would be nice to mention how to run this CI, e.g. that it is automatically triggered in oneDPL repository when a PR is opened and a new commit is made.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is too much detail for what is being requested at this time.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@timmiesmith
Copy link
Collaborator Author

Thank you for the comments, @dmitriy-sobolev . There are several suggestions that I think would be too detailed for this document. Version numbers for drivers and software tools are something that should be captured in oneDPL system requirements documentation.

Comment on lines 111 to 115
Testing on CPU platforms must exercise several execution policies with OpenMP and oneTBB to cover all of the oneDPL backends.
| Processor Type | Execution Policy |
| --- | --- |
| CPU | seq, unseq, par (using oneTBB), par (using OpenMP), par\_unseq(using oneTBB), par\_unseq(using OpenMP) |
| GPU | device\_policy |
Copy link

@dmitriy-sobolev dmitriy-sobolev Apr 22, 2025

Choose a reason for hiding this comment

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

To simplify things, I would replace the 2x2 table with a bullet list:

Suggested change
Testing on CPU platforms must exercise several execution policies with OpenMP and oneTBB to cover all of the oneDPL backends.
| Processor Type | Execution Policy |
| --- | --- |
| CPU | seq, unseq, par (using oneTBB), par (using OpenMP), par\_unseq(using oneTBB), par\_unseq(using OpenMP) |
| GPU | device\_policy |
oneDPL testing must cover all execution policies. Different device types support different sets of policies:
* CPU: seq, unseq, par, par_unseq, device_policy
* GPU: device_policy
The par and par_unseq policies must be tested with both oneTBB and OpenMP backends.

It breaks the flow with its GPU to CPU transition. Adapt the above if you find it useful.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

* CMake
* glibc
* ...
| Windows | Linux | MacOS (Arm CPU testing) |

Choose a reason for hiding this comment

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

Different product on one line still bothers me (such as dpc++ compiler and clang++ compiler). I suggest making a bullet list.

You can ignore this comment though - it is a matter of taste.

Comment on lines 127 to 128
| Visual Studio 2019 | g++ | |
| Visual Studio 2022 | | |

Choose a reason for hiding this comment

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

I think we can leave only one Visual Studio major version (2022) - it should be enough for a minimal CI requirements. I would say that one version is enough in 99% cases.

Also, let's be careful for the name of the product. DPC++ system requirements, for example, list it as Microsoft Visual Studio*.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

napetrov and others added 11 commits April 29, 2025 09:47
…#223)

* introduce initial version of CI descrition

* Apply suggestions from code review

Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>
Co-authored-by: Andreas Huber <9201869+ahuber21@users.noreply.github.com>

* fixing links and typos

* fixing links and typos

* adding sklearnex ci

* adding sklearnex ci

* fixing typo

* Apply suggestions from code review

Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>

* Update project-infrastructure/project-ci-documentation.md

* Update project-infrastructure/project-ci-documentation.md

---------

Co-authored-by: ethanglaser <42726565+ethanglaser@users.noreply.github.com>
Co-authored-by: Andreas Huber <9201869+ahuber21@users.noreply.github.com>
Co-authored-by: Dmitriy Sobolev <Dmitriy.Sobolev@intel.com>
Co-authored-by: Dmitriy Sobolev <Dmitriy.Sobolev@intel.com>
@timmiesmith
Copy link
Collaborator Author

Rebase has mangled the changeset. Closing this PR and will open a clean PR with just the oneDPL CI changes.

@timmiesmith timmiesmith deleted the add_onedpl_ci_info branch May 22, 2025 20:36
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.

oneDPL - Document existing public CI infrastructure and requirements
6 participants