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

[Infer Symbolic Shape No.27][BUAA]edit_distance #67117

Merged
merged 17 commits into from
Aug 12, 2024

Conversation

MufanColin
Copy link
Contributor

PR Category

CINN

PR Types

Others

Description

修改了 edit_distance 算子。因为我刚开始做 CINN,对于相关的内容还不太熟悉,所以这个我先单独提交的。此前给出的修改意见在 #66870 中,我针对修改意见做了修改。

Copy link

paddle-bot bot commented Aug 7, 2024

你的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.


infer_context->AddEqualCstr(hypslength_shape_or_data.shape()[0],
hyps_dims[0]);

Copy link
Contributor

Choose a reason for hiding this comment

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

这个分支里这四个值的0维都是相等的,是不是少了一个约束

Copy link
Contributor Author

Choose a reason for hiding this comment

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

我再添加一个 infer_context->AddEqualCstr(hypslength_shape_or_data, refslength_shape_or_data); 吗?

Copy link
Contributor

Choose a reason for hiding this comment

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

可以

@luotao1 luotao1 added contributor External developers HappyOpenSource Pro 进阶版快乐开源活动,更具挑战性的任务 labels Aug 7, 2024
@MufanColin
Copy link
Contributor Author

这个 Coverage 的测试本地好像测不出来问题,请问后续应该如何修改呢?

@gongshaotian
Copy link
Contributor

test/legacy_test/test_edit_distance_op.py 中 check_pir flag为默认关闭状态,申请approve coverage流水线

}

infer_context->SetShapeOrDataForValue(op->result(0), refs_shape_or_data);
symbol::ShapeOrData<symbol::DimExpr> single_dim_expr({symbol::DimExpr(1)});
Copy link
Contributor

Choose a reason for hiding this comment

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

symbol::ShapeOrDataDimExprs{symbol::TensorShapeOrDataDimExprs(
std::vectorsymbol::DimExpr{DimExpr(1)})} 这地方这么写吧

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 828 to 832
symbol::DimExpr hyps_dim = symbol::DimExpr(hyps_dims[1]);
symbol::DimExpr refs_dim = symbol::DimExpr(refs_dims[1]);

infer_context->AddEqualCstr(hyps_dim, one);
infer_context->AddEqualCstr(refs_dim, one);
Copy link
Contributor

Choose a reason for hiding this comment

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

AddEqualCstr 里的 hyps_dim 可以直接用 hyps_dims[1] 吧?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

收到

infer_context->AddEqualCstr(refs_dim, one);
}

infer_context->SetShapeOrDataForValue(op->result(0), refs_shape_or_data);
Copy link
Contributor

Choose a reason for hiding this comment

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

这样有可能把data也传入,建议再确认下

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 +832 to +836
symbol::ShapeOrDataDimExprs refs_shape_or_data_exprs(
symbol::TensorShapeOrDataDimExprs(
std::vector<symbol::DimExpr>{refs_dims}));
infer_context->SetShapeOrDataForValue(op->result(0),
refs_shape_or_data_exprs);
Copy link
Contributor

Choose a reason for hiding this comment

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

这里命名最好用out_shape_or_data_exprs,out和refs的shape相同,你这里的操作相当于是把refs的shape区取出来给out的符号表达初始化,可以下一个PR顺带修改下

Copy link
Contributor Author

Choose a reason for hiding this comment

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

收到,我下一个 PR 改一下。

@luotao1 luotao1 merged commit 3aaa027 into PaddlePaddle:develop Aug 12, 2024
31 checks passed
MufanColin added a commit to MufanColin/Paddle that referenced this pull request Aug 12, 2024
luotao1 pushed a commit that referenced this pull request Aug 13, 2024
* Finished kldiv loss op

* Finished kldiv loss op

* Fixed name issues

* Resolved comments in #67117
Jeff114514 pushed a commit to Jeff114514/Paddle that referenced this pull request Aug 14, 2024
* Fixd edit_distance op

* Added one more constraint

* Added one more constraint

* Resolved suggested changes

* Added symbol:: before DimExpr

* Fixed errors

* Removed two unnecessary variables and added .shape()

* Fixed typos

* Revert to a previous version

* added .shape()

* added .shape

* added .shape()

* Try to resolve type conflicts

* Try to resolve type conflicts

* Try to resolve type conflicts

* Try to resolve type conflicts
Jeff114514 pushed a commit to Jeff114514/Paddle that referenced this pull request Aug 14, 2024
…Paddle#67327)

* Finished kldiv loss op

* Finished kldiv loss op

* Fixed name issues

* Resolved comments in PaddlePaddle#67117
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers HappyOpenSource Pro 进阶版快乐开源活动,更具挑战性的任务
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants