WIP: Eager mode #491

malmaud · 2019-01-20T19:39:53Z

No description provided.

oxinabox · 2019-01-20T19:59:49Z

src/eager.jl

+end
+
+
+mutable struct TensorHandle


this doesn't need to be mutable does it?

I think it does so that it can have a finalizer.

You might be right there.
Though IIRC you can hook the Finalizer directly to the Ptr

codecov · 2019-01-20T20:31:13Z

Codecov Report

Merging #491 into master will decrease coverage by 0.47%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #491      +/-   ##
==========================================
- Coverage   49.04%   48.56%   -0.48%     
==========================================
  Files          79       80       +1     
  Lines        2659     2685      +26     
==========================================
  Hits         1304     1304              
- Misses       1355     1381      +26

Impacted Files	Coverage Δ
src/TensorFlow.jl	`57.89% <ø> (ø)`	⬆️
src/eager.jl	`0% <0%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b8ff66...ddb4bc7. Read the comment docs.

codecov · 2019-01-20T20:31:13Z

Codecov Report

Merging #491 into master will increase coverage by 4.13%.
The diff coverage is 54.98%.

@@            Coverage Diff             @@
##           master     #491      +/-   ##
==========================================
+ Coverage   49.04%   53.17%   +4.13%     
==========================================
  Files          79       83       +4     
  Lines        2661     2642      -19     
==========================================
+ Hits         1305     1405     +100     
+ Misses       1356     1237     -119

Impacted Files	Coverage Δ
src/py.jl	`0% <0%> (ø)`	⬆️
src/show.jl	`16.34% <0%> (+0.15%)`	⬆️
src/train.jl	`72.29% <0%> (+5%)`	⬆️
src/TensorFlow.jl	`81.25% <100%> (+23.35%)`	⬆️
src/ops/transformations.jl	`93.1% <100%> (+8.72%)`	⬆️
src/version.jl	`92.1% <100%> (+29.36%)`	⬆️
src/ops/summaries.jl	`100% <100%> (ø)`	⬆️
src/generate_ops.jl	`15.1% <15.15%> (-36.07%)`	⬇️
src/eager.jl	`39.58% <39.58%> (ø)`
src/keras.jl	`69.23% <69.23%> (ø)`
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7099f05...0492959. Read the comment docs.

oxinabox · 2019-01-30T21:44:14Z

src/eager.jl

+end
+
+function setindex!(op::EagerOp, dtype::DataType, attr_name)
+    @tfcall(:TFE_OpSetAttrType, Cvoid, (Ptr{Cvoid}, Cstring, TF_DataType), op, attr_name, dtype|>jl_to_df_type)


Suggested change

@tfcall(:TFE_OpSetAttrType, Cvoid, (Ptr{Cvoid}, Cstring, TF_DataType), op, attr_name, dtype|>jl_to_df_type)

tf_dtype = jl_to_df_type(dtype)

@tfcall(:TFE_OpSetAttrType, Cvoid, (Ptr{Cvoid}, Cstring, TF_DataType), op, attr_name, tf_dtype)

oxinabox · 2019-01-30T21:50:05Z

src/eager.jl

+end
+
+function set_attr_list(op::EagerOp, attr_name, list::Vector{<:Integer})
+    list = Int64[Int64(x) for x in list]


Suggested change

list = Int64[Int64(x) for x in list]

list = Int64.(list)

oxinabox · 2019-01-30T21:50:44Z

src/eager.jl

+end
+
+function set_attr_list(op::EagerOp, attr_name, list::Vector{<:AbstractFloat})
+    list = Float32[Float32(x) for x in list]


Suggested change

list = Float32[Float32(x) for x in list]

list = Float32.(x)

oxinabox · 2019-01-30T21:52:57Z

src/eager.jl

+function set_attr_shape_list(op::EagerOp, attr_name, list::Vector)
+    dims = Vector{Int64}[]
+    for shape in list
+        push!(dims, Int64[shape...])


Isn't thiis an error?

I think it's right? At least Keno's PR 9325475 touched this code to make it what it is now.

oxinabox · 2019-01-30T21:54:26Z

src/eager.jl

+    ptr = @tfcall(:TFE_ContextListDevices, Ptr{Cvoid}, (Ptr{Cvoid}, Ptr{Cvoid}), ctx, status)
+    check_status(status)
+    count = @tfcall(:TF_DeviceListCount, Cint, (Ptr{Cvoid},), ptr)
+    this = new(ptr, count)


Can't call new outside of a inner constructor?

malmaud · 2019-01-31T16:39:28Z

Ya all that setattr stuff was jsut copy-pasted from core.jl and blindly tweaked. I suspect a lot of code on this branch will be ugly/buggy as I just try to get a basic system working, at which point I'll worry more about code quality.

oxinabox · 2019-01-31T18:07:51Z

OK, I'll stop bugging about it

Fix transpose. More grads.

oxinabox

Ok, cool cool.
A first heavy pass.
I'ld like to take a closer look still at the import ops generation code.
and also the tape/grads code.

Ideally eager mode would not be a state, but just the result of using eager types.

And I think the gradient code really wants rewriting to be more dispatch based. Since we can dispatch on function types.
So it is like https://github.com/JuliaDiff/DiffRules.jl

src/TensorFlow.jl

src/core.jl

oxinabox · 2019-03-05T23:48:30Z

src/tape.jl

+    return new_tape
+end
+
+function get_tape()


can and thus should be a 1 line function

not applicable anymore since it's no longer a single expression. although if there's a way to write it more compactly, then we should do it.

src/tape.jl

src/summary_writer.jl

malmaud

OK, I responded to some comments. I'll apply some of your patches now, then push a patch, then pick this up a little later. Thanks again for the great feedback.

src/tape.jl

malmaud · 2019-03-15T18:57:26Z

src/summary_writer.jl

+    local path
+    for i in Iterators.countfrom(1)
+        path = joinpath(log_dir, "events.out.tfevents.$i")
+        isfile(path) || break


Don't we want is isfile here? The events file is a file, not a directory. Maybe the variable name should be changed from 'path' if that's confusing.

malmaud · 2019-03-15T18:58:44Z

src/show.jl

@@ -40,12 +40,19 @@ function Base.show(io::IO, t::RawTensor)
    end
 end

+function Base.show(io::IO, t::TensorHandle)
+    raw_tensor = resolve(t)
+    jl_array = convert(Array, raw_tensor)


src/ops/summaries.jl

malmaud · 2019-03-15T19:21:06Z

src/keras.jl

+    lr::TensorHandle
+end
+
+SGD(;lr=1e-3)= SGD(convert(TensorHandle, lr))


This is mirroring keras. Let's make on-boarding easy :)

src/keras.jl

Co-Authored-By: malmaud <malmaud@gmail.com>

malmaud · 2019-03-18T16:03:25Z

Oh ya, good point.

…

On Sat, Mar 16, 2019 at 10:57 AM Lyndon White ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/summary_writer.jl <#491 (comment)>: > mkpath(log_dir) - path = joinpath(log_dir, "events") - pyo = @py_proc pywrap_tensorflow[][:EventsWriter](py_bytes($path)) - writer = FileWriter(pyo, String(log_dir)) + local path + for i in Iterators.countfrom(1) + path = joinpath(log_dir, "events.out.tfevents.$i") + isfile(path) || break As I understand it What we are actually looking for is the first available name that we can use without overwriting anything. The normal thing to be getting in the way is old log files. But it is also not strictly impossible the user has put a directory, a symlink, a mount point or something else there. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#491 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA8SvcJrN6FYp44afCPyxXkuqrTb9dhMks5vXQZUgaJpZM4aJz7O> .

malmaud · 2019-03-18T16:13:01Z

OK, ready for another review. Not everything is ideal, but as long as it's good enough, I would like to merge soon.

oxinabox · 2019-03-18T16:34:11Z

I will try and review this week

malmaud · 2019-03-26T23:11:42Z

I think I'll just merge for now, but I still welcome comments if you get a chance/have the motivation sometime.

oxinabox · 2019-03-27T07:52:13Z

Fair, sorry I didn't get back to this

malmaud requested a review from oxinabox as a code owner January 20, 2019 19:39

oxinabox reviewed Jan 20, 2019

View reviewed changes

malmaud added 5 commits January 30, 2019 14:37

Start work on eager mode

2d1eda8

Trying to get 'execute' working.

b23ff94

Test working

4196715

Attribute setters

e8e0dc1

Variable output

104097a

malmaud force-pushed the eager_mode branch from 8f414b2 to 104097a Compare January 30, 2019 20:01

oxinabox reviewed Jan 30, 2019

View reviewed changes

malmaud added 13 commits February 19, 2019 22:39

Tape AD

de7bafc

fix tape

19161f7

Start generating eager ops

c16ff7a

Test working again

c8ba0e3

Improvements to importing

b55ecd6

Switch to dispatch

8cadecf

casting

43393b8

import more ops

fc3dfb9

Started gradients

a152ca4

Misc.

21bddb0

Fix transpose. More grads.

Diffeq functionality

7d1d071

Keras demo

50d0659

Better grads

42ce802

Disable eager by default

a555ff8

oxinabox reviewed Mar 5, 2019

View reviewed changes

malmaud added 3 commits March 14, 2019 15:13

PyCall adjustments etc.

1c12f64

Overhault context system

3051907

Switch to dispatch for gradients

f7cbd38

oxinabox reviewed Mar 14, 2019

View reviewed changes

src/summary_writer.jl Show resolved Hide resolved

malmaud commented Mar 15, 2019

View reviewed changes

malmaud and others added 15 commits March 15, 2019 15:30

Misc improvements

e05997b

Apply suggestions from code review

f6ef4c7

Co-Authored-By: malmaud <malmaud@gmail.com>

Rename TensorHandle->EagerTensor

fb36d47

Add 'reverse' with dims kw argument

3f6e44e

call_args style

edac1b6

Better fields in Sequential

81dea7d

eliminate some stubs

d4fae80

remove accidental scratch

492d3bf

remove neural ode example until we get it robustly functional

5fda547

Export symbols and change tape.

8cd906f

Bump tf version

8c39cf5

Fix some tests

71e668e

Move @op

ef6b3ab

Tests working

d4a8d5f

Downgrade conda version

f2037bd

Change isfile to ispath in summary_writer

0492959

malmaud merged commit 0635115 into master Mar 26, 2019

oxinabox mentioned this pull request Jan 20, 2020

PyCall not found error #515

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Eager mode #491

WIP: Eager mode #491

malmaud commented Jan 20, 2019

oxinabox Jan 20, 2019

malmaud Jan 20, 2019

oxinabox Jan 20, 2019

codecov bot commented Jan 20, 2019

codecov bot commented Jan 20, 2019 •

edited

Loading

oxinabox Jan 30, 2019

oxinabox Jan 30, 2019

oxinabox Jan 30, 2019

oxinabox Jan 30, 2019

malmaud Mar 15, 2019

oxinabox Jan 30, 2019

malmaud commented Jan 31, 2019

oxinabox commented Jan 31, 2019

oxinabox left a comment

oxinabox Mar 5, 2019

malmaud Mar 15, 2019

malmaud left a comment

malmaud Mar 15, 2019

malmaud Mar 15, 2019

malmaud Mar 15, 2019

malmaud commented Mar 18, 2019 via email

malmaud commented Mar 18, 2019

oxinabox commented Mar 18, 2019

malmaud commented Mar 26, 2019

oxinabox commented Mar 27, 2019

	@tfcall(:TFE_OpSetAttrType, Cvoid, (Ptr{Cvoid}, Cstring, TF_DataType), op, attr_name, dtype\|>jl_to_df_type)
	tf_dtype = jl_to_df_type(dtype)
	@tfcall(:TFE_OpSetAttrType, Cvoid, (Ptr{Cvoid}, Cstring, TF_DataType), op, attr_name, tf_dtype)

WIP: Eager mode #491

WIP: Eager mode #491

Conversation

malmaud commented Jan 20, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jan 20, 2019

Codecov Report

codecov bot commented Jan 20, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

malmaud commented Jan 31, 2019

oxinabox commented Jan 31, 2019

oxinabox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

malmaud left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

malmaud commented Mar 18, 2019 via email

malmaud commented Mar 18, 2019

oxinabox commented Mar 18, 2019

malmaud commented Mar 26, 2019

oxinabox commented Mar 27, 2019

codecov bot commented Jan 20, 2019 •

edited

Loading