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

Struct SparseValue && Bug Fix #31721

Merged
merged 12 commits into from
Apr 7, 2021

Conversation

seiriosPlus
Copy link
Collaborator

@seiriosPlus seiriosPlus commented Mar 18, 2021

PR types

New features

PR changes

Others

Describe

1.修复在分布式预测时,在使用准入策略是,由于未针对预测场景做针对性过滤触发准入条件导致计数及初始化错误.

2.修复在分布式训练时,由于远端拉取稀疏特征时的去重导致Server端对特征的频率计数错误.


Base speed:
{'speed': [83504, 81608, 81701, 83903, 86251]}
PR speed:
'speed': [82247, 85332, 85383, 83559, 86213]


Base AUC:
{'0': 0.7053, '1': 0.7394, '2': 0.7534, '3': 0.7623, '4': 0.7693}}
PR AUC:
{'0': 0.7112, '1': 0.7401, '2': 0.7561, '3': 0.7639, '4': 0.7701}

Change-Id: I516fe974422decaa29c8f1cc70d7ad71f110e732
Change-Id: I4824a4036e1ea5e6da05d66883afe428fe9a339d
Change-Id: I93eedd86d521cc26b05223c3f44c575063e366b3
Change-Id: Idcf48fc9356645d2034e78855dc1a18bb1202cbc
Change-Id: Iacfa0e417ba12293c23add5c17aab9fa00be994f
@paddle-bot-old
Copy link

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@seiriosPlus seiriosPlus changed the title Fix/sparse value struct sparse value Mar 18, 2021
… fix/sparse_value

Change-Id: I57f1d38ebb78d3c4363945f848d15b5f6cbe3b0c
std::vector<int>* offset_shard) const {
offset_shard->reserve(numel_ / shard_num + 1);
for (int x = 0; x < numel_; ++x) {
if (x % shard_num == shard_id) {
Copy link
Contributor

Choose a reason for hiding this comment

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

这样写会有问题,应该是 feasigns_[x] % shard_num == shard_id

Change-Id: I454921269d44892ec174a2fa7b89847deec661bd
@seiriosPlus seiriosPlus changed the title struct sparse value Struct SparseValue && Bug Fix Apr 1, 2021
luotao1
luotao1 previously approved these changes Apr 2, 2021
Copy link
Contributor

@luotao1 luotao1 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 const_cast

Change-Id: Iac18e5dcc6fb363f844b79581f9d09077f2d9288
Change-Id: Ie858cdec6dae2bbe9e907ca3a57d286b704eb450
MrChengmo
MrChengmo previously approved these changes Apr 2, 2021
Copy link
Contributor

@MrChengmo MrChengmo left a comment

Choose a reason for hiding this comment

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

LGTM

Change-Id: Ie35a09772e46f7d90cb68ca82c1d18b9201d1abe
luotao1
luotao1 previously approved these changes Apr 6, 2021
Copy link
Contributor

@luotao1 luotao1 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 const_cast

MrChengmo
MrChengmo previously approved these changes Apr 6, 2021
Copy link
Contributor

@MrChengmo MrChengmo left a comment

Choose a reason for hiding this comment

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

LGTM

// Pull sparse variables from server in sync mode
// pull immediately to tensors
void PullSparseToTensorSync(const uint64_t table_id, int fea_dim,
uint64_t padding_id, platform::Place place,
bool is_training,
Copy link
Collaborator

Choose a reason for hiding this comment

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

add comments please.

Change-Id: I8a06a74f1f482058e2b7479a9a7bd03c6272076e
Change-Id: Icab139e966696b122d331707682351539905dd16
@seiriosPlus seiriosPlus dismissed stale reviews from MrChengmo and luotao1 via e5328a0 April 7, 2021 01:21
Copy link
Contributor

@MrChengmo MrChengmo left a comment

Choose a reason for hiding this comment

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

LGTM

@seiriosPlus seiriosPlus merged commit a881b4d into PaddlePaddle:develop Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants