-
-
Notifications
You must be signed in to change notification settings - Fork 49
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
optimization for the array structure #42
Conversation
the test will not fail in case of a change of the formatting directive
The value is populated on a call to Data()
1 similar comment
For info, there is a bench done in onnx-go |
Gentleman and scholar |
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.
(t *array)
might cause govet to scream at you, other wise all good!
Thank you
|
||
// Pointer returns the pointer of the first value of the slab, as an unsafe.Pointer | ||
func (t *array) Pointer() unsafe.Pointer { return t.Ptr } | ||
func (a *array) Pointer() unsafe.Pointer { return a.Ptr } |
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.
These should be left as t
otherwise govet shouts at you
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.
Strange, I changed it on purpose because go vet was shouting at me, telling me that the name of the receiver was not consistent with the other methods.
I will change it back.
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.
nah I may have an older version of govet. Keep it
|
||
// Data returns the representation of a slice. | ||
func (a array) Data() interface{} { return a.v } | ||
func (a array) Data() interface{} { |
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.
👍 love it
@@ -360,8 +365,11 @@ func copyDenseIter(dst, src DenseTensor, diter, siter Iterator) (int, error) { | |||
return copyDense(dst, src), nil | |||
} | |||
|
|||
if !dst.IsNativelyAccessible() || !src.IsNativelyAccessible() { | |||
return 0, errors.Errorf(inaccessibleData, "copy") | |||
if !dst.IsNativelyAccessible() { |
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.
Thanks for the boyscouting!
@@ -45,10 +46,12 @@ func TestSaveLoadNumpy(t *testing.T) { | |||
t.Error(err) | |||
} | |||
|
|||
expected := "[[ 1. 5.]\n [ 10. -1.]]\n" | |||
expected := `\[\[\s*1\.\s*5\.\]\n \[\s*10\.\s*-1\.\]\]\n` |
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.
Novel test! I like this!
Ok to merge? |
The value field is not initialized when the structure is created.
The population of the value is made when on call to
Data()
if the value isnil