Skip to content

Conversation

@HongyuWangIntel
Copy link
Contributor

@HongyuWangIntel HongyuWangIntel commented Mar 20, 2025

Details:

  • Add an option to apply clamping when converting FP to U8 in NPU SIT post-process.

Tickets:

@github-actions github-actions bot added category: Core OpenVINO Core (aka ngraph) category: CPP API OpenVINO CPP API bindings category: NPU OpenVINO NPU plugin labels Mar 20, 2025
@HongyuWangIntel HongyuWangIntel changed the title [draft] add u8 clamp [draft] Add U8 clamp to NPU SIT Mar 20, 2025
@sys-openvino-ci sys-openvino-ci added the ExternalIntelPR External contributor from Intel label Mar 20, 2025
@github-actions github-actions bot removed category: Core OpenVINO Core (aka ngraph) category: CPP API OpenVINO CPP API bindings labels Mar 28, 2025
@HongyuWangIntel HongyuWangIntel changed the title [draft] Add U8 clamp to NPU SIT [tools][sit] Add U8 clamp to NPU SIT Mar 28, 2025
@HongyuWangIntel HongyuWangIntel marked this pull request as ready for review March 28, 2025 02:15
@HongyuWangIntel HongyuWangIntel requested review from a team as code owners March 28, 2025 02:15
@HongyuWangIntel
Copy link
Contributor Author

Hi, could you please take a look? @ArtemySkrebkov @DariaMityagina @Maxim-Doronin

"In case of one input size: \"[1,3,224,224]\"");
DEFINE_string(skip_output_layers, "" , "Skip output layers from the network. Currently only applicable for"
"RRMSE and NRMSE mode. Accept ';' separated list of output layers");
DEFINE_bool(clamp_u8, false, "Apply clamping when convert fp to u8 to make models like edsr work correctly.");

Choose a reason for hiding this comment

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

We shouldn't mention specific networks.

Suggested change
DEFINE_bool(clamp_u8, false, "Apply clamping when convert fp to u8 to make models like edsr work correctly.");
DEFINE_bool(clamp_u8, false, "Apply clamping when converting FP to U8");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 384f801

std::cout << " Mean_values [channel1,channel2,channel3] " << FLAGS_mean_values << std::endl;
std::cout << " Scale_values [channel1,channel2,channel3] " << FLAGS_scale_values << std::endl;
std::cout << " Skip checking output layers: " << FLAGS_skip_output_layers << std::endl;
std::cout << " Clamp u8 output: " << FLAGS_clamp_u8 << std::endl;

Choose a reason for hiding this comment

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

We should have a more explicit name for the option, if it's only for output:

Suggested change
std::cout << " Clamp u8 output: " << FLAGS_clamp_u8 << std::endl;
std::cout << " Clamp U8 outputs: " << FLAGS_clamp_u8_outputs << std::endl;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 384f801

@Maxim-Doronin
Copy link
Contributor

build_jenkins

@Maxim-Doronin Maxim-Doronin added this pull request to the merge queue Apr 3, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 3, 2025
@Maxim-Doronin Maxim-Doronin added this pull request to the merge queue Apr 3, 2025
Merged via the queue into openvinotoolkit:master with commit 160fa97 Apr 3, 2025
167 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: NPU OpenVINO NPU plugin ExternalIntelPR External contributor from Intel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants