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

Refactor SbpXXX to cfg::SbpXXX #5120

Merged
merged 21 commits into from
Jun 15, 2021
Merged

Refactor SbpXXX to cfg::SbpXXX #5120

merged 21 commits into from
Jun 15, 2021

Conversation

liufengwei0103
Copy link
Contributor

Refactor SbpXXX to cfg::SbpXXX

@oneflow-ci-bot oneflow-ci-bot requested review from oneflow-ci-bot and removed request for oneflow-ci-bot June 8, 2021 04:56
@oneflow-ci-bot oneflow-ci-bot self-requested a review June 8, 2021 06:13
@oneflow-ci-bot oneflow-ci-bot requested review from oneflow-ci-bot and removed request for oneflow-ci-bot June 8, 2021 08:57
@oneflow-ci-bot oneflow-ci-bot requested review from oneflow-ci-bot and removed request for oneflow-ci-bot June 10, 2021 14:42
…low into refactor_sbp_to_cfg_sbp

Conflicts:
	docker/package/manylinux/build_wheel.py
@oneflow-ci-bot oneflow-ci-bot requested review from oneflow-ci-bot and removed request for oneflow-ci-bot June 15, 2021 06:58
@oneflow-ci-bot oneflow-ci-bot merged commit dea9215 into master Jun 15, 2021
@oneflow-ci-bot oneflow-ci-bot deleted the refactor_sbp_to_cfg_sbp branch June 15, 2021 08:04
std::function<Maybe<const SbpInferHint*>(const std::string&)> SbpInferHint4Ibn,
const ParallelDesc& parallel_desc) const override {
CHECK_NE_OR_RETURN(op_conf().cast_to_mirrored_conf().sbp_parallel().parallel_type_case(),
SbpParallel::PARALLEL_TYPE_NOT_SET)
cfg::SbpParallel::PARALLEL_TYPE_NOT_SET)
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里改动完成后 CHECK 的两个参数不一样,一个是 proto 的 enum,一个是 cfg 的 enum,编译器会有警告

parallel_distribution_signature_(
&(kernel_conf.op_attribute().parallel_distribution_signature())) {
parallel_desc_(kernel_conf.op_attribute().parallel_conf_signature().op_parallel_conf()) {
parallel_distribution_signature_ = new cfg::ParallelDistributionSignature(
Copy link
Contributor

Choose a reason for hiding this comment

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

这会内存泄漏吧,还有 124 行

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants