-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
add mine_hard_examples operator #7679
Conversation
: OpProtoAndCheckerMaker(proto, op_checker) { | ||
AddInput( | ||
"ClsLoss", | ||
"(Tensor, default Tensor<float>), The classification loss wit shape " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
"wit shape [N, Np], N is the batch size and Np is the number of " | ||
"prior box.") | ||
.AsDispensable(); | ||
AddInput("MatchIndics", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo error: MatchIndics -> MatchIndices , fix it in all files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
"(Tensor, Tensor<int>), Matched indices with shape [N, Np], N is " | ||
"the batch size and Np is the number of prior box. " | ||
"MatchIndics[i][j] equal -1 means box[j] does not match any " | ||
"entity, otherwise means Box[j] is matched to row."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no box[j]
Box[j]
in context,
MatchIndics[i][j] equal -1 means box[j] does not match any
If MatchIndics[i][j] is -1, it means the j-th prior box in i-th instance does not match any ground-truth box.
otherwise means Box[j] is matched to row."
please also modify this sentence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
"the batch size and Np is the number of prior box. " | ||
"MatchIndics[i][j] equal -1 means box[j] does not match any " | ||
"entity, otherwise means Box[j] is matched to row."); | ||
AddInput("MatchDis", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MatchDis -> MatchDist ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
"(float) The ratio of the negative box to the positive " | ||
"box. Use only when mining_type is equal to max_negative.") | ||
.SetDefault(1.0); | ||
AddAttr<float>("neg_dis_threshold", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dis -> dist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
if (IsEligibleMining(mining_type, match_indices(n, m), match_dis(n, m), | ||
neg_dis_threshold)) { | ||
T loss = cls_loss(n, m); | ||
if (mining_type == MiningType::kHardExample) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (mining_type == MiningType::kHardExample && in_loc_loss) {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
std::vector<int> neg_indices; | ||
for (int n = 0; n < neg_sel; ++n) { | ||
sel_indices.insert(loss_idx[n].second); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::transform
also can be used for this for loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
} | ||
all_neg_indices.push_back(neg_indices); | ||
all_neg_num += neg_indices.size(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The out_neg_indics_lod[0] can be calculated in for (int n = 0; n < batch_size; ++n) {}
, then set LoD like: https://github.com/PaddlePaddle/Paddle/pull/7953/files#diff-7c37cd7e1af079fdcb9b9fef46650007R289
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
for (auto neg_indices : all_neg_indices) { | ||
for (auto neg_idx : neg_indices) { | ||
neg_data[neg_offset++] = neg_idx; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use std::copy
instead of for
loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
"[[1], [0]]."); | ||
|
||
AddOutput("UpdatedMatchIndics", | ||
"(Tensor) The output of updated MatchIndics, a tensor with " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tensor<int>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
} | ||
} | ||
|
||
MiningType GetMiningType(std::string str) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inline MiningType GetMiningType(const std::string& str) {
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
neg_sel = std::min(sample_size, neg_sel); | ||
} | ||
|
||
std::sort(loss_idx.begin(), loss_idx.end(), SortScoreDescend<int>); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loss_idx is std::vector<std::pair<T, size_t>>
, should sorting by SortScoreDescend<T>
not SortScoreDescend<int>
. But I see the origin code in Caffe is also SortScoreDescend<int>
. A little strange.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The T in SortScoreDescend is corresponding the size_t in std::pair<T, size_t>. so SortScoreDescend<size_t> is more reasonable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see. Thanks!
neg_indices.push_back(m); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
从代码效率来讲, line 124 - line 135写成:
if (mining_type == MiningType::kHardExample) {
// ...
} else {
// 直接拷贝 sel_indices中的it.second到neg_indices中即可。这样单测需要注意,neg_indices里的idx顺序有所变化。
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done,
"[N, Np], N is the batch size and Np is the number of prior box."); | ||
AddInput("LocLoss", | ||
"(Tensor, optional, default Tensor<float>), The localization loss " | ||
"wit shape [N, Np], N is the batch size and Np is the number of " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wit -> with
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
"box. Use only when mining_type is equal to max_negative.") | ||
.SetDefault(1.0); | ||
AddAttr<float>("neg_dist_threshold", | ||
"(float) The negative box dis value threshold. " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The negative overlap upper bound for the unmatched predictions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Mine hard examples Operator. | ||
This operator implements hard example mining to select a subset of negative box indices. | ||
For each image, selects the box with highest losses. subject to the condition that the box cannot have | ||
an Matcht > neg_dist_threshold when mining_type is equals max_negative. The selected number is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
subject to the condition that the box cannot have an Matcht > neg_dist_threshold when mining_type is equals max_negative
这句话不是很懂,需要改进下。
when mining_type is equals max_negative -> when mining_type is max_negative .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
an Matcht > neg_dist_threshold when mining_type is equals max_negative. The selected number is | ||
min(sample_size, max_negative_box_number) when mining_type is equals hard_example, | ||
or min(neg_pos_ratio * positive_box_number, max_negative_box_number) when mining_type is | ||
equals max_negative, where the max_negative_box_number is the count of MatchIndices elements with value -1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when mining_type is equals max_negative -> when mining_type is max_negative
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
neg_indices.push_back(m); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 137 to line 142:
neg_indices.reserve(sel_indices.size());
std::transform( sel_indices.begin(), sel_indices.end(),
neg_indices.begin(), [](int d) { return d;});
std::vector<int> neg_indices; | ||
std::transform(loss_idx.begin(), loss_idx.begin() + neg_sel, | ||
std::inserter(sel_indices, sel_indices.begin()), | ||
[](std::pair<T, size_t> l) -> int { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::pair<T, size_t> l
-> std::pair<T, size_t>& l
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved this PR. But the unit test may need to enhance in the future.
resolve #7639