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.

Sign up for GitHub

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

[NPU] Add CLI option to directly pass reference files when running tests #28446

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
+68 −11
Open

[NPU] Add CLI option to directly pass reference files when running tests #28446

Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7280beb
Add CLI option to directly pass reference files when running tests
seowwj Jan 15, 2025
2a00e10
Improve reference files to accept both relative and absolute paths
seowwj Jan 16, 2025
95404ba
Clarify error message to explain when number of ref files does not ma…
seowwj Jan 16, 2025
1a4dacb
Move reference blob path logic to own function
seowwj Jan 20, 2025
4b38006
Merge branch 'openvinotoolkit:master' into npu-SIT-pass-ref-results
seowwj Jan 28, 2025
f2e98a9
Simplify reference files indexing
seowwj Jan 28, 2025
9d6e075
Corrects mistake in ref_results help message
seowwj Jan 28, 2025
3938f80
Using platform agnostic method to construct file path
seowwj Feb 3, 2025
33f81c0
Merge branch 'master' into npu-SIT-pass-ref-results
Maxim-Doronin Feb 10, 2025
abb9035
Merge branch 'master' into npu-SIT-pass-ref-results
Maxim-Doronin Feb 10, 2025
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
79 changes: 68 additions & 11 deletions src/plugins/intel_npu/tools/single-image-test/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ DEFINE_string(
ref_dir,
"",
"A directory with reference blobs to compare with in run_test mode. Leave it empty to use the current folder.");
static constexpr char ref_results_message[] =
"String of reference result file(s) to be used during run_test mode. "
"For the same test case, the files should be separated by comma (,) (example: one case multiple output). "
"For different test cases, it should be separated by semicolon (;). "
"If ref_dir is provided, the reference files should be relative to the ref_dir. "
"Else, if ref_dir is not provided, the reference files should be absolute paths. ";
DEFINE_string(ref_results, "", ref_results_message);
DEFINE_string(mode, "", "Comparison mode to use");

DEFINE_uint32(top_k, 1, "Top K parameter for 'classification' mode");
Expand Down Expand Up @@ -251,8 +258,10 @@ void parseCommandLine(int argc, char* argv[]) {
std::cout << " Scale_values [channel1,channel2,channel3] " << FLAGS_scale_values << std::endl;
std::cout << " Skip checking output layers: " << FLAGS_skip_output_layers << std::endl;
if (FLAGS_run_test) {
std::cout << " Reference files direcotry: "
<< (FLAGS_ref_dir.empty() ? "Current directory" : FLAGS_ref_dir) << std::endl;
std::cout << " Reference files directory: "
<< (FLAGS_ref_dir.empty() && FLAGS_ref_results.empty() ? "Current directory" : FLAGS_ref_dir)
Copy link
Contributor

Choose a reason for hiding this comment

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

from a context it sounds like these parameters are mutually exclusive so that I presume that printing a message and exiting with an error is more intentional

Copy link
Author

Choose a reason for hiding this comment

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

I would argue that they are not mutually exclusive in this case.
Case 1: User provides a reference directory + relative file paths
Result: This line should show the reference directory given

Case 2: User only provides a reference directory
Result: This line should show the reference directory given

Case 3: Nothing provided
Result: This line should show "Current directory"

Copy link
Author

Choose a reason for hiding this comment

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

💡Perhaps it would be even better to show the actual directory path instead of just "current directory"?

Copy link
Contributor

@sivanov-work sivanov-work Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

it's the question about UX: for me mostly as a UNIX user it's quite obvious that if the path starting from "/" it must be an absolute path, so that these parameters --ref_dir=A --ref_outputs=/B;/C mean that /B and /C are expected to be absolute paths rather then A/B and A/C. Thus, if we can configure either absolute or relative paths using only one parameter, instead of changing behavior of that parameter by looking at a value of another one parameter - it will be more natural in usage

this is IMHO, so that maybe it would be better to ask an issue reporter opinion

<< std::endl;
std::cout << " Reference file(s): " << FLAGS_ref_results<< std::endl;
std::cout << " Mode: " << FLAGS_mode << std::endl;
if (strEq(FLAGS_mode, "classification")) {
std::cout << " Top K: " << FLAGS_top_k << std::endl;
Expand Down Expand Up @@ -1822,6 +1831,31 @@ static ov::Shape parseDataShape(const std::string& dataShapeStr) {
return ov::Shape(dataShape);
}

std::string getRefBlobFilePath(const std::string& netFileName, const std::vector<std::string>& refFiles,
size_t numberOfTestCase, size_t outputInd) {
std::string blobFileFullPath;
if (!refFiles.empty() && !FLAGS_ref_dir.empty()) {
// Case 1: Reference files & directory are provided (relative path)
std::filesystem::path refDirPath(FLAGS_ref_dir);
std::filesystem::path refFilePath(refFiles[outputInd]);
blobFileFullPath = (refDirPath / refFilePath).string();
} else if (!refFiles.empty()) {
// Case 2: Reference files provided only (absolute path)
blobFileFullPath = refFiles[outputInd];
} else {
// Case 3: Reference directory provided only
std::ostringstream ostr;
ostr << netFileName << "_ref_out_" << outputInd << "_case_" << numberOfTestCase << ".blob";
const auto blobFileName = ostr.str();

std::filesystem::path fullPath = FLAGS_ref_dir;
fullPath /= blobFileName;
blobFileFullPath = fullPath.string();
}

return blobFileFullPath;
}

static int runSingleImageTest() {
std::cout << "Run single image test" << std::endl;
try {
Expand Down Expand Up @@ -1861,6 +1895,29 @@ static int runSingleImageTest() {
inputFilesForOneInfer.push_back(std::move(entireModelFiles));
}

std::vector<std::string> refFilesPerCase;
using RefFilesPerInput = std::vector<std::string>;
using RefFilesForModelOutputs = std::vector<RefFilesPerInput>;
RefFilesForModelOutputs refFilesForOneInfer;

if (!FLAGS_ref_results.empty()) {
refFilesPerCase = splitStringList(FLAGS_ref_results, ';');
// Make sure that the number of test cases (separated by ;) is the same as number of test cases given in
// input files
if (refFilesPerCase.size() != inputFilesPerCase.size()) {
std::cout << "The number of test cases in reference files is not equal to the number of test cases"
<< " given in input files. "
<< " Number of test cases in reference files: " << refFilesPerCase.size()
<< " Number of test cases in input files: " << inputFilesPerCase.size() << std::endl;
return EXIT_FAILURE;
}

for (const auto& refResult : refFilesPerCase) {
std::vector<std::string> refFilesPerModel = splitStringList(refResult, ',');
Copy link
Contributor

Choose a reason for hiding this comment

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

AFAIR, --network supports only a single model to be loaded. Please explain when we expect several models in a single SIT run.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess that it is for batched input scenario when a whole row looks like CASE_0;CASE_1;etc
where
CASE_X is a run of SIT and which contains INPUT_0,INPUT_1,INPUT2,etc,
where
INPUT_0 encompasses data for single input in batch BATCH_0|BATCH_1|etc
which makes sense for separation input data per model

If my understanding is correct that here we need to produce tensors which are belong to output data types. In case of batch, though inputs pictures for every batch line are separated, outputs are printed into unified single file without separation of its lines. So probably we should not use this level with '|' as perFiles separator

Copy link
Author

Choose a reason for hiding this comment

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

I guess that it is for batched input scenario when a whole row looks like CASE_0;CASE_1;etc where CASE_X is a run of SIT and which contains INPUT_0,INPUT_1,INPUT2,etc, where INPUT_0 encompasses data for single input in batch BATCH_0|BATCH_1|etc which makes sense for separation input data per model

This is correct, the separators are mainly for delimiting if there are multiple expected outputs for the model.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see, but in order to leverage that "multiple expected outputs for the model" you need to write a special logic to split "sigle batched output" onto several non-batched outputs - in order to make it works. Without that logic implemented this "|" is confusing as it doesn't work properly

refFilesForOneInfer.push_back(std::move(refFilesPerModel));
}
}

std::vector<std::string> inputBinPrecisionStrPerCase;
std::vector<std::vector<ov::element::Type>> inputBinPrecisionForOneInfer(inputFilesForOneInfer.size());
if (FLAGS_img_as_bin) {
Expand Down Expand Up @@ -2090,6 +2147,12 @@ static int runSingleImageTest() {
const FilesForModelInputs &inputFiles = inputFilesForOneInfer[numberOfTestCase];
OPENVINO_ASSERT(inputFiles.size() == inputsInfo.size(), "Number of input files ", inputFiles.size(),
" doesn't match network configuration ", inputsInfo.size());
const RefFilesPerInput &refFiles = refFilesForOneInfer.empty() ? RefFilesPerInput{}
: refFilesForOneInfer[numberOfTestCase];
if (!FLAGS_ref_results.empty()) {
OPENVINO_ASSERT(refFiles.size() == outputsInfo.size(), "Number of reference files ", refFiles.size(),
" doesn't match number of network output (s): ", outputsInfo.size());
}

TensorMap inTensors;
size_t inputInd = 0;
Expand Down Expand Up @@ -2169,18 +2232,12 @@ static int runSingleImageTest() {
const ov::element::Type& precision = tensor.get_element_type();
const ov::Shape& shape = tensor.get_shape();

std::ostringstream ostr;
ostr << netFileName << "_ref_out_" << outputInd << "_case_" << numberOfTestCase << ".blob";
const auto blobFileName = ostr.str();

std::filesystem::path fullPath = FLAGS_ref_dir;
fullPath /= blobFileName;
const auto blobFileFullName = fullPath.string();
std::string blobFileFullPath = getRefBlobFilePath(netFileName, refFiles, numberOfTestCase, outputInd);

std::cout << "Load reference output #" << outputInd << " from " << blobFileFullName << " as "
std::cout << "Load reference output #" << outputInd << " from " << blobFileFullPath << " as "
<< precision << std::endl;

const ov::Tensor referenceTensor = loadTensor(precision, shape, blobFileFullName);
const ov::Tensor referenceTensor = loadTensor(precision, shape, blobFileFullPath);
referenceTensors.emplace(tensorName, referenceTensor);

// Determine the output layout
Expand Down
Loading