Skip to content

Simple CUDA Graph migration sample #1545

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
Merged
IgorOchocki merged 10 commits into from
May 10, 2023
Merged

Simple CUDA Graph migration sample #1545

IgorOchocki merged 10 commits into from
May 10, 2023

Conversation

@ShwethaSelma
Copy link
Contributor

@ShwethaSelma ShwethaSelma commented Apr 12, 2023

Adding a New Sample(s)

Description

SimpleCudaGraphs migration from CUDA to SYCL and mapping of Taskflow API's to CUDA Graph API's.

Checklist

Administrative

  • Review sample design with the appropriate Domain Expert: Choudary, Chekuri; Krishnappa, Rakshith
  • If you have any new dependencies/binaries, inform the oneAPI Code Samples Project Manager

Code Development

Security and Legal

  • OSPDT Approval (see Project Manager for assistance)
  • Compile using the following compiler flags and fix any warnings, the falgs are: "/Wall -Wformat-security -Werror=format-security"
  • Bandit Scans (Python only)
  • Virus scan

Review

  • Review DPC++ code with Paul Peterseon. (GitHub User: pmpeter1)
  • Review readme with Tom Lenth(@tomlenth) and/or Project Manager
  • Tested using Dev Cloud when applicable

@ShwethaSelma ShwethaSelma mentioned this pull request Apr 12, 2023
Closed
15 tasks
@jimmytwei jimmytwei requested review from a user and ishaghosh27 April 14, 2023 20:31
@jimmytwei
Copy link
Contributor

jimmytwei commented Apr 14, 2023

Adding @jkinsky and @ishaghosh27 for README review.

ghost
ghost suggested changes Apr 17, 2023
View reviewed changes
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

Sample name in sample.json file should match the sample name in readme.

@jimmytwei
Copy link
Contributor

jimmytwei commented Apr 18, 2023

@ShwethaSelma Verified sample builds and runs. Please incorporate @jkinsky feedback to the sample and then I will approve and merge it.

@ShwethaSelma
Copy link
Contributor Author

ShwethaSelma commented Apr 18, 2023

@ShwethaSelma Verified sample builds and runs. Please incorporate @jkinsky feedback to the sample and then I will approve and merge it.

I have incorporated the comment and updated the sample name in sample.json file.

@jimmytwei jimmytwei requested a review from tomlenth April 19, 2023 21:51
@jimmytwei
Copy link
Contributor

jimmytwei commented Apr 19, 2023

@ShwethaSelma I think the sample name should be something more descriptive than "SimpleCudaGraphs Sample". Adding @tomlenth and @jkinsky for feedback.

Also, it is missing the expertise field in sample.json. Adding @ishaghosh27 for feedback.

@ghost
Copy link

ghost commented Apr 20, 2023

@ShwethaSelma I think the sample name should be something more descriptive than "SimpleCudaGraphs Sample". Adding @tomlenth and @jkinsky for feedback.

Also, it is missing the expertise field in sample.json. Adding @ishaghosh27 for feedback.

Agree. At the very least, you should change the name in the readme and sample.json files to "Simple Cuda Graphs" Sample. Adding spaces will make it better even if you don't change the name completely.

@ishaghosh27
Copy link
Contributor

ishaghosh27 commented Apr 20, 2023

@ShwethaSelma @jimmytwei Please add Expertise: Code Optimization to the sample.json file.

@ShwethaSelma
Copy link
Contributor Author

ShwethaSelma commented Apr 24, 2023

@ShwethaSelma @jimmytwei Please add Expertise: Code Optimization to the sample.json file.

This sample doesn't include code optimization, should i update the expertise as "Concept and Functionality"?

@ShwethaSelma
Copy link
Contributor Author

ShwethaSelma commented Apr 24, 2023

@ShwethaSelma I think the sample name should be something more descriptive than "SimpleCudaGraphs Sample". Adding @tomlenth and @jkinsky for feedback.
Also, it is missing the expertise field in sample.json. Adding @ishaghosh27 for feedback.

Agree. At the very least, you should change the name in the readme and sample.json files to "Simple Cuda Graphs" Sample. Adding spaces will make it better even if you don't change the name completely.

I have changed the name as suggested.

@ishaghosh27
Copy link
Contributor

ishaghosh27 commented Apr 24, 2023

@ShwethaSelma @jimmytwei Please add Expertise: Code Optimization to the sample.json file.

This sample doesn't include code optimization, should i update the expertise as "Concept and Functionality"?

Yes please update expertise as "Concept and Functionality"

@ShwethaSelma
Copy link
Contributor Author

ShwethaSelma commented Apr 25, 2023

Concept and Functionality"

Updated.

@IgorOchocki IgorOchocki merged commit cc85129 into oneapi-src:development May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ishaghosh27 ishaghosh27 Awaiting requested review from ishaghosh27

@tomlenth tomlenth Awaiting requested review from tomlenth

1 more reviewer
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

2023.2 ci-ready

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ShwethaSelma @jimmytwei @ishaghosh27 @IgorOchocki