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

Fix bug error msg format #5866

Merged
merged 16 commits into from
Aug 17, 2021
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion oneflow/core/common/error.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ Error Error::InputDeviceNotMatchError() {
auto error = std::make_shared<cfg::ErrorProto>();
auto* input_device_not_match_error = error->mutable_input_device_not_match_error();
input_device_not_match_error->add_info(
std::string("The devices of input tensors are inconsistentplease try to use tensor.to or "
std::string("The devices of input tensors are inconsistent, please try to use tensor.to or "
"module.to to correct it."));
return error;
}
Expand Down
140 changes: 98 additions & 42 deletions oneflow/core/common/error_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ limitations under the License.
*/
#include <sstream>
#include "oneflow/core/common/error_util.h"
#include "oneflow/core/common/util.h"

namespace oneflow {

Expand All @@ -24,8 +25,8 @@ std::string StripSpace(std::string str) {
if (str.size() == 0) { return ""; }
size_t pos = str.find_first_not_of(" ");
if (pos != std::string::npos) { str.erase(0, pos); }
pos = str.find_last_not_of(" ") + 1;
if (pos != std::string::npos) { str.erase(pos); }
pos = str.find_last_not_of(" ");
if (pos != std::string::npos) { str.erase(pos + 1); }
return str;
}

Expand All @@ -41,92 +42,147 @@ std::string StripBrackets(std::string str) {
return str;
}

Maybe<std::string> ShortenErrorMsg(std::string str) {
Maybe<std::string> ShortenMsg(std::string str) {
// 150 characters is the threshold
const int num_displayed_char = 150;
const int character_num_threshold = 150;
const int displayed_char_num = 50;
if (str.size() == 0) { return str; }
// strip space when JUST( xx );
str = StripSpace(str);
if (str.size() < num_displayed_char) { return str; }

// Find first index where the number of characters from the start to the index is less than 50,
// last index is the same
int first_index = -1;
int last_index = -1;
int pre_index = 0;
CHECK_OR_RETURN(str.size() >= 1);
for (int i = 1; i < str.size(); i++) {
if (IsLetterNumberOrUnderline(str.at(i)) && !IsLetterNumberOrUnderline(str.at(i - 1))) {
if (first_index == -1 && i >= num_displayed_char / 3) { first_index = pre_index; }
if (last_index == -1 && str.size() - i <= num_displayed_char / 3) { last_index = i; }
pre_index = i;
if (str.size() < character_num_threshold) { return str; }

// left part whose number of characters is just over 50
int left_index = displayed_char_num;
if (IsLetterNumberOrUnderline(str.at(left_index))) {
for (; left_index < str.size(); left_index++) {
if (!IsLetterNumberOrUnderline(str.at(left_index))) { break; }
}
} else {
for (; left_index < str.size(); left_index++) {
if (IsLetterNumberOrUnderline(str.at(left_index))) { break; }
}
}
// A string of more than 150 characters
if (first_index == -1 && last_index == -1) { return str; }
CHECK_OR_RETURN(first_index <= str.size());
CHECK_OR_RETURN(last_index <= str.size());
std::stringstream ss;
// The number of characters before the first word exceeds 50
if (first_index == -1) {
ss << " ... " << str.substr(last_index);
}
// The number of characters after the last word exceeds 50
else if (last_index == -1) {
ss << str.substr(0, first_index) << " ... ";

// right part whose number of characters is just over 50
int right_index = str.size() - displayed_char_num;
if (IsLetterNumberOrUnderline(str.at(right_index))) {
for (; right_index >= 0; right_index--) {
if (!IsLetterNumberOrUnderline(str.at(right_index))) {
right_index++;
break;
}
}
} else {
ss << str.substr(0, first_index) << " ... " << str.substr(last_index);
for (; right_index >= 0; right_index--) {
if (IsLetterNumberOrUnderline(str.at(right_index))) {
right_index++;
break;
}
}
}
// a long word of more than 150
if (right_index - left_index < 50) { return str; }
std::stringstream ss;
CHECK_OR_RETURN(left_index >= 0);
CHECK_OR_RETURN(left_index < str.size());
ss << str.substr(0, left_index);
ss << " ... ";
CHECK_OR_RETURN(right_index >= 0);
CHECK_OR_RETURN(right_index < str.size());
ss << str.substr(right_index);
return ss.str();
Copy link
Contributor

Choose a reason for hiding this comment

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

这里是否需要包一层CHECK?

}

std::string FormatFile(const std::string& file) {
// file info in stack frame
std::string FormatFileOfStackFrame(const std::string& file) {
std::stringstream ss;
ss << "\n File \"" << file << "\", ";
return ss.str();
}

std::string FormatLine(const int64_t& line) {
// line info in stack frame
std::string FormatLineOfStackFrame(const int64_t& line) {
std::stringstream ss;
ss << "line " << line << ",";
return ss.str();
}

std::string FormatFunction(const std::string& function) {
// function info in stack frame
std::string FormatFunctionOfStackFrame(const std::string& function) {
std::stringstream ss;
ss << " in " << function;
return ss.str();
}

Maybe<std::string> FormatErrorMsg(std::string error_msg, bool is_last_stack_frame) {
// msg in stack frame
Maybe<std::string> FormatMsgOfStackFrame(std::string error_msg, bool is_last_stack_frame) {
error_msg = StripBrackets(error_msg);
if (!is_last_stack_frame) { error_msg = *JUST(ShortenErrorMsg(error_msg)); }
if (!is_last_stack_frame) { error_msg = *JUST(ShortenMsg(error_msg)); }
// error_msg of last stack frame come from "<<"
if (is_last_stack_frame) { error_msg = StripSpace(error_msg); }
std::stringstream ss;
ss << "\n " << error_msg;
return ss.str();
}

std::string FormatErrorSummaryAndMsg(const std::shared_ptr<cfg::ErrorProto>& error) {
// the error_summary and msg in error proto
std::string FormatErrorSummaryAndMsgOfErrorProto(const std::shared_ptr<cfg::ErrorProto>& error) {
std::stringstream ss;
if (error->has_error_summary()) { ss << error->error_summary(); }
if (error->has_msg()) { ss << (ss.str().size() != 0 ? ", " + error->msg() : error->msg()); }
if (error->has_msg()) { ss << (ss.str().size() != 0 ? "\n" + error->msg() : error->msg()); }
return ss.str();
}

// the msg in error type instance.
Maybe<std::string> FormatMsgOfErrorType(const std::shared_ptr<cfg::ErrorProto>& error) {
std::stringstream ss;
CHECK_NE(error->error_type_case(), cfg::ErrorProto::ERROR_TYPE_NOT_SET);
switch (error->error_type_case()) {
case cfg::ErrorProto::kInputDeviceNotMatchError:
ss << error->input_device_not_match_error().DebugString();
break;
case cfg::ErrorProto::kMemoryZoneOutOfMemoryError:
ss << error->memory_zone_out_of_memory_error().DebugString();
break;
case cfg::ErrorProto::kMultipleOpKernelsMatchedError:
ss << error->multiple_op_kernels_matched_error().DebugString();
break;
case cfg::ErrorProto::kOpKernelNotFoundError:
ss << error->op_kernel_not_found_error().DebugString();
break;
case cfg::ErrorProto::kConfigResourceUnavailableError:
ss << error->config_resource_unavailable_error().DebugString();
break;
case cfg::ErrorProto::kConfigAssertFailedError:
ss << error->config_assert_failed_error().DebugString();
Copy link
Contributor

Choose a reason for hiding this comment

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

可以看下这个可不可以用反射实现,这样手动列出每一种情况是不可维护的,用反射的话伪代码大概是这样:

ss << PbMessage2Txt(error->getMessage(error->error_type_name()))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

可以看下这个可不可以用反射实现,这样手动列出每一种情况是不可维护的,用反射的话伪代码大概是这样:

ss << PbMessage2Txt(error->getMessage(error->error_type_name()))

已修改采用反射实现

break;
default: ss << "";
}
return ss.str();
}

} // namespace

Maybe<std::string> FormatErrorStr(const std::shared_ptr<cfg::ErrorProto>& error) {
std::stringstream ss;
// Get msg from stack frame of error proto
for (auto stack_frame = error->mutable_stack_frame()->rbegin();
stack_frame < error->mutable_stack_frame()->rend(); stack_frame++) {
ss << FormatFile(*stack_frame->mutable_file()) << FormatLine(*stack_frame->mutable_line())
<< FormatFunction(*stack_frame->mutable_function())
<< *JUST(FormatErrorMsg(*stack_frame->mutable_error_msg(),
stack_frame == error->mutable_stack_frame()->rend() - 1));
ss << FormatFileOfStackFrame(*stack_frame->mutable_file())
<< FormatLineOfStackFrame(*stack_frame->mutable_line())
<< FormatFunctionOfStackFrame(*stack_frame->mutable_function())
<< *JUST(FormatMsgOfStackFrame(*stack_frame->mutable_error_msg(),
stack_frame == error->mutable_stack_frame()->rend() - 1));
}
ss << "\n" << FormatErrorSummaryAndMsg(error);
// Get msg from error summary and msg of error proto
std::string error_summary_and_msg_of_error_proto = FormatErrorSummaryAndMsgOfErrorProto(error);
if (error_summary_and_msg_of_error_proto.size() != 0) {
ss << "\n" << error_summary_and_msg_of_error_proto;
}
// Get msg from error type of error proto
std::string msg_of_error_type = *JUST(FormatMsgOfErrorType(error));
if (msg_of_error_type.size() != 0) { ss << "\n" << msg_of_error_type; }

return ss.str();
Copy link
Contributor

Choose a reason for hiding this comment

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

这里是否也要check下

}

Expand Down
2 changes: 0 additions & 2 deletions oneflow/core/common/error_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ namespace oneflow {
namespace cfg {
class ErrorProto;
}
std::string* MutErrorStr();
const std::string& GetErrorStr();
Maybe<std::string> FormatErrorStr(const std::shared_ptr<cfg::ErrorProto>& error);
} // namespace oneflow

Expand Down