-
Notifications
You must be signed in to change notification settings - Fork 157
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
[res/tfl_recipes] Add new Net_FullyConnected_Mul #13650
[res/tfl_recipes] Add new Net_FullyConnected_Mul #13650
Conversation
This commit extends Net_FullyConnectedMul tflite recipes with 'no bias' case. ONE-DCO-1.0-Signed-off-by: Jan Iwaszkiewicz <[email protected]>
@@ -0,0 +1,13 @@ | |||
# This checks if: | |||
# Mul(FC(input, weights, _), other) |
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.
there is bias
in the test.recipe
file but you've noted as _
which I understand this as no bias. This is confusing.
if 003
is with bias, I think we need another 004
without bias.
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.
There is no bias here, it's a special test case requested here: #13607 (comment)
The third input of FC is empty:
ONE/res/TensorFlowLiteRecipes/Net_FullyConnected_Mul_003/test.recipe
Lines 36 to 46 in e3ff517
operation { | |
type: "FullyConnected" | |
fullyconnected_options { | |
activation: NONE | |
keep_num_dims: true | |
} | |
input: "ifm" | |
input: "fc_wgt" | |
input: "" | |
output: "fc_out" | |
} |
The B
operand is the input of Mul
(as in A * B
):
ONE/res/TensorFlowLiteRecipes/Net_FullyConnected_Mul_003/test.recipe
Lines 47 to 55 in e3ff517
operation { | |
type: "Mul" | |
mul_options { | |
activation: RELU | |
} | |
input: "fc_out" | |
input: "B" | |
output: "mul_out" | |
} |
Is it okay or is there another notation for empty bias?
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.
Ah, thanks for the explanation.
"B"
made the confusion.
It would be nice if you renamed B
as mutiplicand
or scale
.
Anyway, is there existing case WITH BIAS? if not, can you add one?
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.
#13610 -- here are already merged models, all of them have fc_bias
.
It would be nice if you renamed B as mutiplicand or scale.
Will do!
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.
LGTM
This commit extends Net_FullyConnectedMul tflite recipes with 'no bias' case.
ONE-DCO-1.0-Signed-off-by: Jan Iwaszkiewicz [email protected]