-
Notifications
You must be signed in to change notification settings - Fork 78
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
tensorflow test #378
tensorflow test #378
Conversation
Best reviewed: commit by commit
Optimal code review plan (2 warnings)
|
Codecov Report
@@ Coverage Diff @@
## master #378 +/- ##
=========================================
+ Coverage 7.35% 8.29% +0.94%
=========================================
Files 383 372 -11
Lines 19664 18847 -817
=========================================
+ Hits 1446 1564 +118
+ Misses 17999 17068 -931
+ Partials 219 215 -4
Continue to review full report at Codecov.
|
}, | ||
}, | ||
{ | ||
name: "set value", |
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.
name: "set value", | |
name: "set success when opts is not nil", |
{ | ||
name: "set value", | ||
args: args{ | ||
opts: &SessionOptions{}, |
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.
opts: &SessionOptions{}, | |
opts: new(SessionOptions), |
}, | ||
want: want{ | ||
obj: &T{ | ||
options: &SessionOptions{}, |
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.
options: &SessionOptions{}, | |
options: new(SessionOptions), |
}(), | ||
*/ | ||
{ | ||
name: "set default", |
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.
name: "set default", | |
name: "set nothing when tgt is empty", |
*/ | ||
{ | ||
name: "set default", | ||
args: args{ |
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.
if you want to set empty or nil object, you can delete args
field.
args: args {
tgt: ""
}
↑ you can delete `args` fields
}(), | ||
*/ | ||
{ | ||
name: "return (value, nil)", |
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.
name: "return (value, nil)", | |
name: "returns (value, nil) when run function returns nil", |
checkFunc: defaultCheckFunc, | ||
}, | ||
{ | ||
name: "run() error", |
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.
name: "run() error", | |
name: "returns (nil, error) when run function returns `session.Run() error`", |
checkFunc: defaultCheckFunc, | ||
}, | ||
{ | ||
name: "nil tensor error: run() return nil", |
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.
name: "nil tensor error: run() return nil", | |
name: "returns (nil, error) when | |
tensors returned by the run function is nil", |
}, | ||
{ | ||
name: "nil tensor error: run() return nil", | ||
args: args{ |
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.
Could you please delete this field.
checkFunc: defaultCheckFunc, | ||
}, | ||
{ | ||
name: "nil tensor error: run() return [nil]", |
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.
name: "nil tensor error: run() return [nil]", | |
name: "returns (nil, error) when | |
element of tensors returned by the run function is nil", |
checkFunc: defaultCheckFunc, | ||
}, | ||
{ | ||
name: "nil tensor error: run() return nil", |
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.
name: "nil tensor error: run() return nil", | |
name: "returns (nil, error) when | |
tensors returned by the run function is nil", |
checkFunc: defaultCheckFunc, | ||
}, | ||
{ | ||
name: "nil tensor error: run() return [nil]", |
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.
name: "nil tensor error: run() return [nil]", | |
name: "returns (nil, error) when | |
element of tensors returned by the run function is nil", |
checkFunc: defaultCheckFunc, | ||
}, | ||
{ | ||
name: "failed to cast error: ndim == TwoDim", |
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.
name: "failed to cast error: ndim == TwoDim", | |
name: "returns (nil, error) when ndim is `TwoDim` and returns error of `ErrFailedToCastTF", |
checkFunc: defaultCheckFunc, | ||
}, | ||
{ | ||
name: "failed to cast error: ndim == ThreeDim", |
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.
name: "failed to cast error: ndim == ThreeDim", | |
name: "returns (nil, error) when ndim is `ThreeDim ` and returns error of`ErrFailedToCastTF`", |
checkFunc: defaultCheckFunc, | ||
}, | ||
{ | ||
name: "failed to cast error: ndim == default", |
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.
name: "failed to cast error: ndim == default", | |
name: "returns (nil, error) when ndim is `default ` and returns error of`ErrFailedToCastTF`", |
*/ | ||
{ | ||
name: "return ([], nil): inputs == nil", | ||
args: args{ |
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.
you can delete this field.
}(), | ||
*/ | ||
{ | ||
name: "return ([], nil): inputs == nil", |
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.
name: "return ([], nil): inputs == nil", | |
name: "return ([], nil) when inputs is nil and Run function returns ([], nil)", |
checkFunc: defaultCheckFunc, | ||
}, | ||
{ | ||
name: "length error", |
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.
name: "length error", | |
name: "returns (nil, error) when length of inputs and feeds field are different", |
checkFunc: defaultCheckFunc, | ||
}, | ||
{ | ||
name: "session.Run() error", |
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.
name: "session.Run() error", | |
name: "returns (nil, error) when Run function returns error", |
checkFunc: defaultCheckFunc, | ||
}, | ||
{ | ||
name: "return ([], nil): inputs == {\"test\"}", |
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.
name: "return ([], nil): inputs == {\"test\"}", | |
name: "return ([], nil) when inputs is `{"test"} and Run function returns ([], nil)`", |
}, | ||
err: nil, | ||
}, | ||
checkFunc: defaultCheckFunc, |
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.
you can delete this line, because this function was set automatically in the tt.Run
.
/rebase |
1 similar comment
/rebase |
[REBASE] Rebase triggered by rinx for branch: test/internal/tensorflow |
243a570
to
044658c
Compare
CloseFunc func() error | ||
} | ||
|
||
func (m *mockSession) Run(feeds map[tf.Output]*tf.Tensor, fetches []tf.Output, operations []*Operation) ([]*tf.Tensor, error) { |
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.
[golangci] reported by reviewdog 🐶
line is 127 characters (lll)
options: nil, | ||
graph: nil, | ||
session: &mockSession{ | ||
RunFunc: func(feeds map[tf.Output]*tf.Tensor, fetches []tf.Output, operations []*tf.Operation) ([]*tf.Tensor, error) { |
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.
[golangci] reported by reviewdog 🐶
line is 123 characters (lll)
options: nil, | ||
graph: tf.NewGraph(), | ||
session: &mockSession{ | ||
RunFunc: func(feeds map[tf.Output]*tf.Tensor, fetches []tf.Output, operations []*tf.Operation) ([]*tf.Tensor, error) { |
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.
[golangci] reported by reviewdog 🐶
line is 123 characters (lll)
options: nil, | ||
graph: tf.NewGraph(), | ||
session: &mockSession{ | ||
RunFunc: func(feeds map[tf.Output]*tf.Tensor, fetches []tf.Output, operations []*tf.Operation) ([]*tf.Tensor, error) { |
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.
[golangci] reported by reviewdog 🐶
line is 123 characters (lll)
@@ -56,20 +63,17 @@ const ( | |||
ThreeDim | |||
) | |||
|
|||
var loadFunc = func(exportDir string, tags []string, options *SessionOptions) (*tf.SavedModel, error) { |
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.
[golangci] reported by reviewdog 🐶
loadFunc
is a global variable (gochecknoglobals)
want: want{ | ||
obj: &T{ | ||
feeds: []OutputSpec{ | ||
OutputSpec{ |
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.
[golangci] reported by reviewdog 🐶
File is not gofmt
-ed with -s
(gofmt)
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.
@datelier
Could you please format this file?
exportDir: "", | ||
tags: nil, | ||
feeds: []OutputSpec{ | ||
OutputSpec{ |
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.
[golangci] reported by reviewdog 🐶
File is not gofmt
-ed with -s
(gofmt)
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.
@@ -109,7 +113,7 @@ func (t *tensorflow) GetVector(inputs ...string) ([]float64, error) { | |||
if err != nil { | |||
return nil, err | |||
} | |||
if tensors == nil || tensors[0] == nil || tensors[0].Value() == nil { | |||
if len(tensors) == 0 || tensors[0] == nil || tensors[0].Value() == nil { |
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.
[golangci] reported by reviewdog 🐶
if statements should only be cuddled with assignments (wsl)
@@ -149,7 +153,7 @@ func (t *tensorflow) GetValue(inputs ...string) (interface{}, error) { | |||
if err != nil { | |||
return nil, err | |||
} | |||
if tensors == nil || tensors[0] == nil { | |||
if len(tensors) == 0 || tensors[0] == nil { |
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.
[golangci] reported by reviewdog 🐶
if statements should only be cuddled with assignments (wsl)
func New(opts ...Option) (TF, error) { | ||
t := new(tensorflow) | ||
for _, opt := range append(defaultOpts, opts...) { | ||
opt(t) | ||
} | ||
|
||
if t.options == nil && (len(t.sessionTarget) != 0 || t.sessionConfig != nil) { | ||
if t.options == nil && (t.sessionTarget == "" || t.sessionConfig != nil) { |
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.
we don't need nil
checking for the string.
you can try on https://play.golang.org/p/_9aagB_YbOM
if t.options == nil && (t.sessionTarget == "" || t.sessionConfig != nil) { | |
if t.options == nil && t.sessionTarget == "" { |
After I run the go test command, the following error occurred.
after searching on google i found this link: do we need to install the tool before running the go test? |
@kevindiu |
if len(tgt) != 0 { | ||
t.sessionTarget = tgt | ||
if tgt != "" { | ||
if t.options == nil { |
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.
I think it's good! 👍
/rebase |
[REBASE] Rebase triggered by datelier for branch: test/internal/tensorflow |
b4a0498
to
4b8448d
Compare
@@ -56,25 +63,25 @@ const ( | |||
ThreeDim | |||
) | |||
|
|||
var loadFunc = func(exportDir string, tags []string, options *SessionOptions) (*tf.SavedModel, error) { |
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.
[golangci] reported by reviewdog 🐶
unlambda: replace `func(exportDir string, tags []string, options *SessionOptions) (*tf.SavedModel, error) {
I tried to install it but it is failed....
|
Seems the make command is not supported on Mac, I installed the |
/rebase |
[REBASE] Rebase triggered by vankichi for branch: test/internal/tensorflow |
[REBASE] Failed to rebase. |
Signed-off-by: datelier <57349093+datelier@users.noreply.github.com>
17304e0
to
bb536a8
Compare
LGTMed other than https://github.com/vdaas/vald/pull/378/files#r429748517 |
Signed-off-by: datelier <57349093+datelier@users.noreply.github.com>
LGTM |
/rebase |
[REBASE] Rebase triggered by kevindiu for branch: test/internal/tensorflow |
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
Signed-off-by: datelier <57349093+datelier@users.noreply.github.com>
) | ||
|
||
var loadFunc = tf.LoadSavedModel |
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.
[golangci] reported by reviewdog 🐶
loadFunc
is a global variable (gochecknoglobals)
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
LGTM |
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
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
Description:
test tensorflow package.
Related Issue:
How Has This Been Tested?:
Environment:
Types of changes:
Changes to Core Features:
Checklist: