backport basic refactor from PR#100 #103
Conversation
|
It mainly did two things:
|
|
@phipsgabler @devmotion we are now entering the design and performance optimisation phase of the new tape mechanism since the functionality has been proven correct (TuringLang/Turing.jl#1757). Please comment on any issues so that we can gradually build a lightweight, performant, maintainable tape implementation. The tape implementation will be moved into |
| for instruction in tape | ||
| instruction() | ||
| increase_counter(tape) | ||
| end | ||
| end |
There was a problem hiding this comment.
Intuitively, I'd have expected this to also return the result of the function. Or is that extracted in another place?
There was a problem hiding this comment.
The result is stored in Instruction.output.
| tape.owner = tf | ||
| return tf | ||
| end | ||
|
|
||
| function Base.show(io::IO, tf::TapedFunction) | ||
| buf = IOBuffer() |
There was a problem hiding this comment.
Why the extra buffer? You could directly println to io.
There was a problem hiding this comment.
Here I use an extra buffer, because that there are too many calls to println with io (which is stdout in most situations). And between these calls, the current task may be switched off, then Juia does some print in another task, and then switched on. In that case, the output will not be continuous and that makes it annoying to read.
There was a problem hiding this comment.
Good reason. Then put a comment there to remind future readers why this should not be refactored.
Besides adding
TapeInstruction, I did a little code refactor in #100, and I'd like to move that refactor onto master branch before doing benchmark and performance optimization.