Fdy/support hf lora amp #308
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
fandaoyi
commented
Sep 22, 2023
•
fandaoyi
commented
- 重写了部分 torch/csrc/device 的逻辑，dipu 可以在python层显示为 ‘cuda’ (包括 device， print tensor， generator， storage 都会显示 ‘device=cuda’)。 并fix了部分受影响的ci 和其他部分（主要是storage） （camb 的test 受影响比较多，主要是目前test 的设备还是 ’dipu‘导致的， 另外 改为cuda 会多跑一些测试。 后续计划先使用 DIPU_PYTHON_DEVICE_AS_CUDA= false 暂时绕过。 等凌劼的 ci梳理后一起处理。）
- 新增了一个弱符号接口 getDeviceStatus() 用于获取 mem_info 等设备实时信息， 另外对原来的 getDevicePropertiesFromCache 和新接口做了一定程度的统一。
- 删除了部分多余的 generator 逻辑（已经和蔡坤同步过）。
Conflicts: dipu/torch_dipu/__init__.py
fandaoyi
requested review from
caikun-pjlab,
zhaoguochun1995,
mrdanielw and
lljbash
caikun-pjlab
approved these changes
Oct 9, 2023
lljbash
approved these changes
Oct 9, 2023
lljbash
previously requested changes
Oct 9, 2023
| return THPUtils_packString(oss.str().c_str()); | ||
| } | ||
|
|
||
| static struct PyGetSetDef DIPU_THPDevice_properties[] = { |
There was a problem hiding this comment.
应该不能用 const， 后面要赋值给 tp_getset， 那个不是 const的。
There was a problem hiding this comment.
这居然是个数组- -看起来更可怕了...
好像也没啥好改法，要不然加个注释说这玩意儿传给 pytorch 随他用去了
lljbash
reviewed
Oct 9, 2023
lljbash
reviewed
Oct 9, 2023
| using dipu::devapis::DIPUDeviceProperties; | ||
| using c10::DeviceIndex; | ||
|
|
||
| DeviceIndex num_gpus = -1; | ||
| c10::once_flag init_flag; | ||
| std::deque<c10::once_flag> device_flags; | ||
| std::vector<DIPUDeviceProperties> device_properties; | ||
| std::vector<shared_ptr<DIPUDeviceProperties>> device_properties; |
There was a problem hiding this comment.
原来的逻辑会把裸指针传到 python层。用 shared_ptr pybind11 可以和cpp层共同管理 生命周期。
There was a problem hiding this comment.
所以这个对象在 python 层可能会活得比 cpp 层久嘛
There was a problem hiding this comment.
应该不会。但是之前传一个裸指针过去， 还要改变 pybind 默认的析构行为 py::return_value_policy::reference，还是有些奇怪的。也不太符合 pybind 推荐的模式
mrdanielw
approved these changes
Oct 10, 2023
fandaoyi
dismissed
lljbash’s stale review
seems remaining comments are enhanced suggestions, not mandatory。 they shouldn’t be ‘request’ but ‘comments’
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.