RSDK-4196 Add support for flat tensors in mlmodel #122
Conversation
| @@ -10,6 +10,7 @@ | |||
| # performance-move-const-arg: TODO(RSDK-3019) | |||
| # misc-unused-parameters: TODO(RSDK-3008) remove this and fix all lints. | |||
| # readability-function-cognitive-complexity: No, complexity is subjective and sometimes necessary. | |||
| # readability-else-after-return: No, this causes code complexification | |||
There was a problem hiding this comment.
This clang-tidy rule makes the following a lint error:
if (foo) {
return x;
} else if (bar) {
return y;
} else {
return z;
}
And requires that it be written:
if (foo) {
return x;
}
if (bar) {
return y;
}
return z;
Which just feels unnecessary. I think I get what they are trying to do, which is to avoid the following:
if (!result) {
return error;
} else {
return success;
}
In favor of:
if (!result) {
return error;
}
return success;
Which is a change i would generally support. But it doesn't work well at all for the big ladders of conditionals I need to write to handle the various type cases in this PR. I tried living with it and it caused me distress so I disabled the check.
If others feel differently I can restore the check and make the mechanical changes to placate the linter.
| mlmodel_details::tensor_storage storage; | ||
| MLModelService::named_tensor_views views; | ||
| }; | ||
| constexpr bool kUseFlatTensors = true; |
There was a problem hiding this comment.
I've opted to leave the existing struct based implementation in place so I can easily A/B test. All code selected by kUseFlatTensors = false would be removable if we decided to make a breaking change.
| @@ -12,6 +12,9 @@ | |||
| // See the License for the specific language governing permissions and | |||
| // limitations under the License. | |||
|
|
|||
| #include <memory> | |||
There was a problem hiding this comment.
I've moved these down where they belong.
| if (!data || (size == 0) || shape.empty()) { | ||
| std::ostringstream message; | ||
| message << "Empty or zero length data or shape"; | ||
| throw std::invalid_argument(message.str()); |
There was a problem hiding this comment.
(q) just for my own clarity, what is message doing here? Naively it looks like we're adding a string and then immediately unpacking that string.
There was a problem hiding this comment.
@stuqdog - Merely copy paste from other places where I was adding some state to the string, but here I'm not. I'll remove it.
acmorrow
changed the title
Updates the C++ SDK to support the changes from viamrobotics/api#279.