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

NPUW: Added conditional checks for accuracy for spatial subgraphs #27348

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ DEFINE_OPT(NPUW_FUNCALL_ASYNC, bool, false, npuw::funcall_async, RunTime);
DEFINE_OPT(NPUW_ACC_CHECK, bool, false, npuw::accuracy::check, RunTime);
DEFINE_OPT(NPUW_ACC_THRESH, double, 0.01, npuw::accuracy::threshold, RunTime);
DEFINE_OPT(NPUW_ACC_DEVICE, std::string, "", npuw::accuracy::reference_device, RunTime);
DEFINE_OPT(NPUW_ACC_DUMP_FAILS, bool, false, npuw::accuracy::dump_failures, RunTime);
DEFINE_OPT(NPUW_DUMP_FULL, bool, false, npuw::dump::full, CompileTime);
DEFINE_OPT(NPUW_DUMP_SUBS, std::string, "", npuw::dump::subgraphs, CompileTime);
DEFINE_OPT(NPUW_DUMP_SUBS_ON_FAIL, std::string, "", npuw::dump::subgraphs_on_fail, CompileTime);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,14 @@ static constexpr ov::Property<double> threshold{"NPUW_ACC_THRESH"};
* Default value: empty.
*/
static constexpr ov::Property<std::string> reference_device{"NPUW_ACC_DEVICE"};

/**
* @brief
* Type: bool.
* Enable dumps of materials for model(s), failing accuracy check.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please rephrase. What are the "materials"?
Also, as we operate at subgraph level, there's no "model(s)"

* Default value: false.
*/
static constexpr ov::Property<std::string> dump_failures{"NPUW_ACC_DUMP_FAILS"};
} // namespace accuracy

namespace dump {
Expand Down
1 change: 1 addition & 0 deletions src/plugins/intel_npu/src/al/src/config/npuw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ void intel_npu::registerNPUWOptions(OptionsDesc& desc) {
desc.add<NPUW_ACC_CHECK>();
desc.add<NPUW_ACC_THRESH>();
desc.add<NPUW_ACC_DEVICE>();
desc.add<NPUW_ACC_DUMP_FAILS>();
#ifdef NPU_PLUGIN_DEVELOPER_BUILD
desc.add<NPUW_DUMP_FULL>();
desc.add<NPUW_DUMP_SUBS>();
Expand Down
64 changes: 44 additions & 20 deletions src/plugins/intel_npu/src/plugin/npuw/accuracy/comparator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,30 +13,47 @@
ov::npuw::metrics::NRMSE::NRMSE(double threshold) : m_threshold(threshold) {}

bool ov::npuw::metrics::NRMSE::operator()(const ov::SoPtr<ov::ITensor>& actual,
const ov::SoPtr<ov::ITensor>& reference) const {
NPUW_ASSERT(actual->is_continuous());
NPUW_ASSERT(reference->is_continuous());
const ov::SoPtr<ov::ITensor>& reference,
double* result) const {
Comment on lines 15 to +17
Copy link
Contributor

Choose a reason for hiding this comment

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

This bool operator() (.., double *result); thing looks weird. Just make a proper return type like Result.

For those who were only checking true/false, that type could have operator bool().

NPUW_ASSERT(actual->get_shape() == reference->get_shape());
// Check for alignment:
NPUW_ASSERT(actual->get_byte_size() == reference->get_byte_size());
// FIXME: Check for strides

ov::Tensor in_actual(actual->get_element_type(), actual->get_shape());
ov::Tensor in_reference(reference->get_element_type(), reference->get_shape());

if (!actual->is_continuous()) {
ov::make_tensor(actual).copy_to(in_actual);
} else {
in_actual = ov::make_tensor(actual);
}
if (!reference->is_continuous()) {
ov::make_tensor(reference).copy_to(in_reference);
} else {
in_reference = ov::make_tensor(reference);
}

// TODO: it might be more correct to make to_f32 function
// to work with strided tensors
NPUW_ASSERT(in_actual.is_continuous());
NPUW_ASSERT(in_reference.is_continuous());

ov::Tensor actual_f32;
ov::Tensor reference_f32;

if (ov::element::Type_t::f32 == actual->get_element_type()) {
actual_f32 = ov::make_tensor(actual);
if (ov::element::f32 == in_actual.get_element_type()) {
actual_f32 = in_actual;
} else {
ov::Tensor dst(ov::element::Type_t::f32, actual->get_shape());
ov::npuw::util::to_f32(ov::make_tensor(actual), dst);
ov::Tensor dst(ov::element::Type_t::f32, in_actual.get_shape());
ov::npuw::util::to_f32(in_actual, dst);
Copy link
Contributor

Choose a reason for hiding this comment

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

since you anyways convert to f32, it could be easier to teach this to_f32 function work with strided tensors - at least for inputs.
May be worth a todo and a follow-up ticket.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added TODO, will create a ticket!

actual_f32 = std::move(dst);
}

if (ov::element::Type_t::f32 == reference->get_element_type()) {
reference_f32 = ov::make_tensor(reference);
if (ov::element::f32 == in_reference.get_element_type()) {
reference_f32 = in_reference;
} else {
ov::Tensor dst(ov::element::Type_t::f32, reference->get_shape());
ov::npuw::util::to_f32(ov::make_tensor(reference), dst);
ov::Tensor dst(ov::element::Type_t::f32, in_reference.get_shape());
ov::npuw::util::to_f32(in_reference, dst);
reference_f32 = dst;
}

Expand All @@ -51,13 +68,21 @@ bool ov::npuw::metrics::NRMSE::operator()(const ov::SoPtr<ov::ITensor>& actual,
}

if (squared_error <= std::numeric_limits<double>::epsilon()) {
LOG_INFO("NRMSE loss: 0.0, threshold: " << m_threshold << ".");
LOG_INFO("PASS");
if (result != nullptr) {
*result = 0.0;
}
return true;
}

double rmse = sqrt(squared_error / size);
NPUW_ASSERT(rmse >= 0.0);

if (rmse < 0.0) {
// Calculated RMSE metric is < 0.0, what is unexpected. So, return that tensors are unequal.
if (result != nullptr) {
*result = rmse;
}
return false;
}

auto actual_min_max = std::minmax_element(actual_data, actual_data + size);
auto reference_min_max = std::minmax_element(reference_data, reference_data + size);
Expand All @@ -66,9 +91,8 @@ bool ov::npuw::metrics::NRMSE::operator()(const ov::SoPtr<ov::ITensor>& actual,
std::max(0.f, *actual_min_max.second) - std::min(0.f, *actual_min_max.first)});

double nrmse = rmse / den;
LOG_INFO("NRMSE loss: " << nrmse << ", threshold: " << m_threshold << ".");

bool success = nrmse <= m_threshold;
LOG_INFO((success ? "PASS" : "FAIL"));
return success;
if (result != nullptr) {
*result = nrmse;
}
return nrmse <= m_threshold;
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ namespace metrics {
class NRMSE {
public:
explicit NRMSE(double threshold);
bool operator()(const ov::SoPtr<ov::ITensor>& backup_tensor, const ov::SoPtr<ov::ITensor>& original_tensor) const;

bool operator()(const ov::SoPtr<ov::ITensor>& backup_tensor,
const ov::SoPtr<ov::ITensor>& original_tensor,
double* result = nullptr) const;
private:
double m_threshold{};
};
Expand Down
Loading
Loading