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.

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

赛题七-开发grad_fn、next_functions两个API 并暴露到python端-v1 #54838

Merged
merged 6 commits into from
Jul 11, 2023

Conversation

@paddle-bot
Copy link

paddle-bot bot commented Jun 25, 2023

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels Jun 25, 2023
@qiuwenbogdut qiuwenbogdut changed the title 开发grad_fn、next_functions两个API 并暴露到python端-v1 赛题七-开发grad_fn、next_functions两个API 并暴露到python端-v1 Jun 25, 2023
@@ -559,4 +559,19 @@ void GradNodeBase::HandleComplexGradToRealGrad(
}
}

std::vector<GradNodeBase*> GradNodeBase::NextFunctions() {
Copy link
Contributor

Choose a reason for hiding this comment

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

这里返回指针,而不是智能指针是不安全的。如果node析构了,而Python端或者别的地方还持有std::vector<GradNodeBase*>,可能造成非法访问

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::vector<std::shared_ptr<GradNodeBase>>
std::vector<std::shared_ptr<GradNodeBase>> GradNodeBase::NextFunctions() {
    std::vector<std::shared_ptr<GradNodeBase>> next_nodes;
    const paddle::small_vector<std::vector<GradSlotMeta>, kSlotSmallVectorSize>& metas = OutputMeta();
    
    for (const auto& meta_list : metas) {
        for (const GradSlotMeta& meta : meta_list) {
            const auto& edge = meta.GetEdge();
            // 获取关联的智能指针而非裸指针
            std::shared_ptr<GradNodeBase> next_node = edge.GetGradNode();
            next_nodes.push_back(next_node);
        }
    }
    
    return next_nodes;
}

修改成这样使用智能指针是否可行?

Comment on lines 254 to 255
//增加一个方法 返回一个固定值1 作为测试
int64_t GetNextHookId() { return 1; }
Copy link
Contributor

Choose a reason for hiding this comment

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

这个是测试用的吗?如果是最后得删掉哈~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个是测试用的吗?如果是最后得删掉哈~

是的, 后面删掉

//增加一个方法 返回一个固定值1 作为测试
int64_t GetNextHookId() { return 1; }

// 定义一个方法实现访问当前gradnode的下一个gradnode
Copy link
Contributor

Choose a reason for hiding this comment

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

咱们框架里最好不要有中文注释啊改成英文吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

咱们框架里最好不要有中文注释啊改成英文吧

好的 后面注释使用英文

Comment on lines 288 to 293
PyObject* tensor_properties_qiutest(TensorObject* self, void* closure) {
EAGER_TRY
int grad_fn_value = 1;
return ToPyObject(grad_fn_value);
EAGER_CATCH_AND_THROW_RETURN_NULL
}
Copy link
Contributor

Choose a reason for hiding this comment

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

这个应该是测试代码吧?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个应该是测试代码吧?

是的 需要删除

Comment on lines 308 to 309
// 打印一下meta
std::cout << "meta is " << meta << std::endl;
Copy link
Contributor

Choose a reason for hiding this comment

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

std::cout是不能出现在正式代码里的啊。

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::cout是不能出现在正式代码里的啊。

好的,下次注意

auto grad_node = meta->GradNode(); // Convert GradNode to a Python object
// The conversion will depend on the structure of GradNode.

// 实现一下将gradnode 对象转化成python对象
Copy link
Contributor

Choose a reason for hiding this comment

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

后面所有的中文注释都处理一下吧~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

后面所有的中文注释都处理一下吧~

好的

Comment on lines 322 to 326
//获取python 转换器
py::object py_obj = py::cast(grad_node, py::return_value_policy::reference);
py::handle py_handle = py::handle(py_obj);
PyObject* py_grad_node = py_handle.ptr();
Py_INCREF(py_grad_node); // 增加引用计数,以确保在返回时不会被销毁
Copy link
Contributor

Choose a reason for hiding this comment

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

把这段逻辑封装一个ToPyobject吧

@@ -320,6 +373,8 @@ struct PyGetSetDef variable_properties[] = {
{"dtype", (getter)tensor_properties_get_dtype, nullptr, nullptr, nullptr},
{"type", (getter)tensor_properties_get_type, nullptr, nullptr, nullptr},
{"is_leaf", (getter)tensor_properties_is_leaf, nullptr, nullptr, nullptr},
{"grad_fn", (getter)tensor_properties_get_grad_fn, nullptr, nullptr, nullptr},
{"qiutest", (getter)tensor_properties_qiutest, nullptr, nullptr, nullptr},
Copy link
Contributor

Choose a reason for hiding this comment

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

测试代码删一下

Copy link
Contributor Author

Choose a reason for hiding this comment

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

测试代码删一下

ok


py::class_<egr::GradNodeBase>(m, "GradNodeBase")
//GetNextHookId 暴露一个函数 一直返回一个固定的值
.def("get_next_hook_id", &egr::GradNodeBase::GetNextHookId)
Copy link
Contributor

Choose a reason for hiding this comment

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

这个是干嘛用的?我没太理解~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这个是干嘛用的?我没太理解~
这个也需要删除的

@paddle-ci-bot
Copy link

paddle-ci-bot bot commented Jul 3, 2023

Sorry to inform you that 5e2ba5c's CIs have passed for more than 7 days. To prevent PR conflicts, you need to re-run all CIs manually.

@CLAassistant
Copy link

CLAassistant commented Jul 3, 2023

CLA assistant check
All committers have signed the CLA.

Comment on lines 570 to 571
// std::shared_ptr<GradNodeBase>
// next_node(edge.GetMutableGradNode().get());
Copy link
Contributor

Choose a reason for hiding this comment

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

这两行注释删掉吧

Comment on lines 290 to 296
PyObject* convert_grad_node_to_py_object(egr::GradNodeBase* grad_node) {
py::object py_obj = py::cast(grad_node, py::return_value_policy::reference);
py::handle py_handle = py::handle(py_obj);
PyObject* py_grad_node = py_handle.ptr();
Py_INCREF(py_grad_node);
return py_grad_node;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

这个函数重命名成ToPyObject。放到paddle/fluid/pybind/eager_utils.h和paddle/fluid/pybind/eager_utils.cc中吧

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的 已经处理

@luotao1
Copy link
Contributor

luotao1 commented Jul 7, 2023

#54838 (comment)
同时解决下CLA的签署问题

@qiuwenbogdut
Copy link
Contributor Author

#54838 (comment) 同时解决下CLA的签署问题
好的 已经解决

wanghuancoder
wanghuancoder previously approved these changes Jul 7, 2023
Copy link
Contributor

@wanghuancoder wanghuancoder left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@lanxianghit lanxianghit left a comment

Choose a reason for hiding this comment

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

LGTM for Python API

@wanghuancoder wanghuancoder merged commit ab46b14 into PaddlePaddle:develop Jul 11, 2023
@luotao1
Copy link
Contributor

luotao1 commented Jul 11, 2023

hi, @qiuwenbogdut

  • 非常感谢你对飞桨框架的贡献,我们正在运营一个PFCC组织,会通过定期分享技术知识与发布开发者主导任务的形式持续为飞桨框架做贡献,详情可见 https://github.com/luotao1 主页说明。
  • 如果你对PFCC有兴趣,请发送邮件至 [email protected],我们会邀请你加入~

cqulilujia pushed a commit to cqulilujia/Paddle that referenced this pull request Jul 24, 2023
* [尝试] 给tensor增加一个属性, 这个属性是一个定值 1

* 暴露gradnode 并构建gradnode新的方法(用来测试)进行暴露给python python端可以访问

* 开发grad_fn、next_functions两个API 并暴露到python端- 做一些规范化处理

* 增加一个单元测试

* 优化 code-style
wz1qqx pushed a commit to wz1qqx/Paddle that referenced this pull request Jul 31, 2023
* [尝试] 给tensor增加一个属性, 这个属性是一个定值 1

* 暴露gradnode 并构建gradnode新的方法(用来测试)进行暴露给python python端可以访问

* 开发grad_fn、next_functions两个API 并暴露到python端- 做一些规范化处理

* 增加一个单元测试

* 优化 code-style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants