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

feat(dipu): implement setter/getter for offset&seed of diopiGenerator #762

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

feat(dipu): implement setter/getter for offset&seed of diopiGenerator #762

wants to merge 9 commits into from

Conversation

Bonbon-Tang
Copy link
Collaborator

@Bonbon-Tang Bonbon-Tang commented Apr 1, 2024 •
edited
Loading

增加 offset和seed获取
DeepLink-org/DIOPI#1127

lljbash
lljbash reviewed Apr 1, 2024
View reviewed changes
Copy link
Collaborator

@lljbash lljbash left a comment

Choose a reason for hiding this comment

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

建议 description 里面关联一下 diopi 的更新，如有必要，还得更新 diopi

dipu/torch_dipu/csrc_dipu/diopirt/diopirt_impl.cpp Outdated
Comment on lines 190 to 193
{
offset = gen_impl->get_offset();
seed = gen_impl->current_seed();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个 scope 是为什么？下同

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

DIOPI那边需要拿到这两个参数

Copy link
Collaborator

Choose a reason for hiding this comment

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

为什么有个大括号 scope？

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

这个我研究下，之前应该写了线程锁

Copy link
Collaborator

Choose a reason for hiding this comment

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

不需要的话就去掉吧

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

应该不需要 我去掉了

@Bonbon-Tang Bonbon-Tang changed the title add seed and offset tyf/add seed and offset Apr 9, 2024
@lljbash lljbash changed the title tyf/add seed and offset feat(dipu): implement setter/getter for offset and seed of diopiGenerator Apr 10, 2024
@lljbash lljbash changed the title feat(dipu): implement setter/getter for offset and seed of diopiGenerator feat(dipu): implement setter/getter for offset&seed of diopiGenerator Apr 10, 2024
@lljbash lljbash added the DIPU DIPU related label Apr 10, 2024
xuq7410 pushed a commit to xuq7410/deeplink.framework that referenced this pull request May 23, 2024
@zhaoguochun1995
Zgc/diopi ascend fix python gil bug (DeepLink-org#762)
70ed3df 
* delete python relate depandcy

* optimize the location of releasing python GIL lock

* add Python.h dir to include path

* rename gil_scoped_release to GilScopedRelease

---------

Co-authored-by: zhaoguochun1995 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lljbash lljbash lljbash approved these changes

@mrdanielw mrdanielw Awaiting requested review from mrdanielw mrdanielw is a code owner

@fandaoyi fandaoyi Awaiting requested review from fandaoyi fandaoyi is a code owner

At least 2 approving reviews are required to merge this pull request.

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

Successfully merging this pull request may close these issues.
2 participants
@Bonbon-Tang @lljbash