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
Support Transpose op in TFlite #25297
base: 4.x
Are you sure you want to change the base?
Conversation
@CNOCycle, please open a PR to https://github.com/opencv/opencv_extra/ with the same branch name as here. |
Hi, @dkurt, Thank you for your valuable feedback. After careful consideration, I have decided to split this PR into two separate ones. The first will focus on implementing shape checkers, while the second will address the support for the Transpose op. Regarding the Transpose op support, I have chosen to postpone it until we resolve the shape issues. There are a couple of reasons for this decision. Firstly, I need additional time to seamlessly integrate my stand-alone script for generating testing data into the existing script in the repository Specifically, I added the shape checker as you suggested (
These errors are reproducible on RISC-V RVV with Debug mode and x64 with Release mode. The introduced shape checker effectively identifies the inconsistent shape issue. Do you have any insights or suggestions regarding these errors? |
@CNOCycle, this is a data layout issue I mentioned before. TFLite/TensorFlow work with NHWC by default, OpenCV with NCHW. So during a layer import you have to change axes order: std::vector<int> perm = allTensors[op.inputs()->Get(1)];
DataLayout inpLayout = layouts[op.inputs()->Get(0)];
if (inpLayout == DNN_LAYOUT_NHWC && perm.size() == 4) {
static const int order[] = {0, 2, 3, 1}; // NHWC -> NCHW
for (int& dim: perm) {
CV_Assert(dim >= 0 && dim < 4);
dim = order[dim];
}
} If in your test example |
Thank you for providing a clear explanation of the data layout issue. It's important to note that the failed tests I mentioned earlier are not new tests related to the Transpose op; they are existing tests in the opencv repo. Upon direct inspection of the .tflite models, I observed that the shapes of the output tensors from |
@CNOCycle, thanks for the observation about shapes. I verified that test data for both models were saved in native view, without necessary reshaping. We can fix it by updating test data but I prefer to just add a workaround in test engine: ASSERT_EQ(outs.size(), outNames.size());
for (int i = 0; i < outNames.size(); ++i) {
Mat ref = blobFromNPY(findDataFile(format("dnn/tflite/%s_out_%s.npy", modelName.c_str(), outNames[i].c_str())));
if (modelName == "face_landmark" || modelName == "selfie_segmentation") {
ref = ref.reshape(1, 1);
outs[i] = outs[i].reshape(1, 1);
}
normAssert(ref, outs[i], outNames[i].c_str(), l1, lInf);
} Note that |
@CNOCycle I merged another patch for TFLite tests and it generates conflict. Could you rebase your PR and fix the conflict. |
Apologies for the delayed response. I encountered a link error while attempting to build the scalable RVV on Debug mode from the latest |
f920e1e
to
6e262ff
Compare
@dkurt is it ready for merge? |
else if (perm[1] == 3 && perm[2] == 2 && perm[3] == 1) { | ||
std::vector<int> orderLP = {0, 2, 1, 3}; | ||
layerParams.set("order", DictValue::arrayInt<int*>(orderLP.data(), orderLP.size())); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, change layout of output:
opencv/modules/dnn/src/tensorflow/tf_importer.cpp
Line 1339 in 197626a
void TFImporter::parseTranspose(tensorflow::GraphDef& net, const tensorflow::NodeDef& layer, LayerParams& layerParams) |
} | ||
if (perm[1] == 1 && perm[2] == 2 && perm[3] == 3) { | ||
std::vector<int> orderLP = {0, 1, 2, 3}; | ||
layerParams.set("order", DictValue::arrayInt<int*>(orderLP.data(), orderLP.size())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reduce code duplications
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @dkurt, Thank you for the valuable feedback. Concerning the code complexity issues, it's essential to note that the transpose operation in TF implementation also encompasses six cases to handle data layout. opencv/modules/dnn/src/tensorflow/tf_importer.cpp Lines 1354 to 1396 in 197626a
The rationale behind specifying these six cases is elucidated in the comment:
To ensure alignment with the NCHW layout, merely applying a NHWC -> NCHW conversion to permutation vector is insufficient. The sequence of the order vector varies based on the given permutation vector. Allow me to illustrate what I mean through the following demonstration. Suppose we have a vector A = [N, H, W, C] and a permutation vector P = [0, 1, 2, 3]. After the transpose operation, the output should be [N, H, W, C], while the expected format for OpenCV is [N, C, H, W]. Once the input is represented in a channel-first format, denoted as At = [N, C, H, W], the permutation vector should be adjusted accordingly. Applying NHWC -> NCHW conversion to the permutation vector, it becomes Pt = [0, 3, 1, 2]. However, applying Pt on At results in [N, W, C, H], which is an incorrect outcome. As evidenced, when providing At = [N, C, H, W] with P = [0, 1, 2, 3], the expected output is [N, C, H, W], resulting in the order vector being set to [0, 1, 2, 3]. The mapping of the order vector for six cases is shown below: A = [N, H, W, C], At = [N, C, H, W] |
|
Hi @dkurt Is there any progress on this PR? |
|
@CNOCycle, please consider review comments |
I have fixed the tab issue and provided 5 test cases to verify correctness. If still have any concerns about this PR, please let me know. Thanks. |
@dkurt could you take a look again? |
sorry for late reply. I will re-submit a revised one today. |
Pull Request Readiness Checklist
Merge with extra: opencv/opencv_extra#1168
The purpose of this PR is to introduce support for the Transpose op in TFlite format and to add a shape comparison between the output tensors and the references. In some occasional cases, the shape of the output tensor is
[1,4,1,1]
, while the shape of the reference tensor is[1,4]
. Consequently, the norm check incorrectly reports that the test has passed, as the residual is zero.Below is a Python script for generating testing data. The generated data can be integrated into the repo
opencv_extra
.Patch to opencv_extra has the same branch name.