Dc evaluate perf #2932
Dc evaluate perf #2932
Conversation
jakesabathia2
requested review from
abhishekpratapa,
shantanuchhabra,
guihao-liang and
nickjong
| flex_list class_labels = read_state<flex_list>("classes"); | ||
| auto max_prob_label = [=](const flexible_type& ft) { | ||
| const flex_vec& prob_vec = ft.get<flex_vec>(); | ||
| auto max_it = std::max_element(prob_vec.begin(), prob_vec.end()); |
There was a problem hiding this comment.
Is the use of auto here increasing readability?
There was a problem hiding this comment.
Yes. And also follow other code's style.
|
pass internal test. |
| gl_sarray predictions_prob = predict(data, "probability_vector"); | ||
|
|
||
| flex_list class_labels = read_state<flex_list>("classes"); | ||
| auto max_prob_label = [=](const flexible_type& ft) { |
There was a problem hiding this comment.
The indentation level is not correct. Run clang-format if possible.
| class_label = evaluation_result["prediction_class"] | ||
| probability_vector = evaluation_result["prediction_prob"] | ||
|
|
||
| del evaluation_result["prediction_class"] |
There was a problem hiding this comment.
you don't have to delete those. They will be collected by GC.
|
Sorry for this late review. @jakesabathia2, I think it's better to discover why predict class is slower than before. |
Predict is faster tam 5.8 tbh. |
|
Duplicate the logic inside is not an optimal design. Besides, be careful about the code style. |
I am not duplicating that logic. The duplication in previous code is what I fixed in this PR. |
That's what I mean. A better way to do it is to have a subroutine (inline function, for example) to wrap the same logic so that you will have only one place to edit in the future if there's a need to modify that logic. |
good! |
|
To put it in a simple way, make it more maintainable is also a key point for good code. |
good! will put an other pr. |
fix #2803