JuliaDynamics / ResumableFunctions.jl

C# style generators a.k.a. semi-coroutines for Julia.
Other
158 stars 19 forks source link

Proposal: drop/ignore return value rather than yielding it #15

Closed garrison closed 6 years ago

garrison commented 6 years ago

Hi there

I'm very excited about ResumableFunctions.jl. It is a wonderful demonstration of the power of julia's macro system.

On the other hand, I find one design choice to be puzzling: the choice to yield the final/return value of a resumable function as an additional value. I propose that instead, all such values should be @yielded explicitly, and any such return value should be dropped.

This would have a number of advantages:

See also issue #13, which may be related.

garrison commented 6 years ago

Also related to #2.

BenLauwens commented 6 years ago

Hi

I am aware of this "feature";)

I use @resumable function as a way to abstract the finite-state machine when doing process driven event-driven simulation. The yielding of the final/return value is a safe-guard for the end-users of SimJulia who are not necessarily aware what is happening behind the @resumable macro. A process stops when the process function reaches a normal return. The yielding of an event, means that their is still something to do afterwards.

A more important problem is how your proposal can be implemented. When a value is yielded (implemented as a normal return) the @resumable function can not know that this is the final value. Only when a real return is encountered the @resumable function stops iterating. In C# the behaviour is identical and this comes from the way how it is implemented.

In Julia a function returns always something (nothing) so I can not create a @resumable function that yields no value. In Python a return without argument is allowed to stop the generator, but this is implemented at a language level allowing this behaviour...

So I am a little bit stuck, I understand your motivations for this modification but I see no way how to implement it.

If you have any ideas how to do this. Feel free to share it and I am willing to integrate it in the package.

Kind regards

Ben

garrison commented 6 years ago

Hi Ben,

Thank you for the reply.

I have only scratched the surface so far, but let me mention a few things that come to mind. I think Python is able to deal with this issue in the way I expect because its iteration protocol relies on the StopIteration exception, which can be thrown upon iteration when there are no more values. It seems that the Revised iteration protocol Julep (which addresses JuliaLang/julia#8149) may actually solve the underlying issue here, which would then make this proposal possible.

cortner commented 6 years ago

just to say I also just tried this package and probably have to give up on it because of this. Not being able to ignore the final return makes it much much more tedious to use. (otherwise I love it!) unfortunately I don't have an idea either how to do this.

BenLauwens commented 6 years ago

Please check out branch Python-generators. Can you confirm that this is the behaviour you really like? If so I will update documentation and release v0.2;) Kind regards

Ben

BenLauwens commented 6 years ago

Only on Julia v0.7 this is supported... I will back port if asked for

BenLauwens commented 6 years ago

I did a quick back-port to Julia v0.6, so check out branch Python-generators-v0.6

cortner commented 6 years ago

Thank you, I’ll try to test this tonight.

garrison commented 6 years ago

Thank you!

One difference between this and python's generators is that the values are always calculated one ahead of what is currently necessary. For instance,

@resumable function f(n)
    for i in 1:n
        println("yielding $i")
        @yield i
    end
end

for a in f(4)
    println(a)
end

yields

yielding 1
yielding 2
1
yielding 3
2
yielding 4
3
4

This may be a reasonable stopgap measure, though, until JuliaLang/julia#18823 is addressed.

I also noticed one issue when specifying the type of the return value, which I believe is a surmountable implemention issue:

@resumable function f(n)::Int
    for i in 1:n
        @yield i
    end
end

collect(f(4))
ERROR: MethodError: Cannot `convert` an object of type Nothing to an object of type Int64
BenLauwens commented 6 years ago

Hi

I found inspiration in the new Julia iteration protocol that behaves identically: values are always calculated one ahead.

When specifying the return type. The function has to return the correct type... because values are calculated one ahead. A simple solution can be:

@resumable function f(n)::Int
    for i in 1:n
        @yield i
    end
    0
end

collect(f(4))

I do some further testing and if no other problems are raised I will release somewhere next week an updated version.

cortner commented 6 years ago

After checking out the new branch: (on v0.6)

julia> using ResumableFunctions
INFO: Precompiling module ResumableFunctions.
ERROR: LoadError: LoadError: UndefVarError: KeySet not defined
Stacktrace:
 [1] include_from_node1(::String) at ./loading.jl:576
 [2] include(::String) at ./sysimg.jl:14
 [3] include_from_node1(::String) at ./loading.jl:576
 [4] include(::String) at ./sysimg.jl:14
 [5] anonymous at ./<missing>:2
while loading /Users/ortner/.julia/v0.6/ResumableFunctions/src/transforms.jl, in expression starting on line 521
while loading /Users/ortner/.julia/v0.6/ResumableFunctions/src/ResumableFunctions.jl, in expression starting on line 14
ERROR: Failed to precompile ResumableFunctions to /Users/ortner/.julia/lib/v0.6/ResumableFunctions.ji.
Stacktrace:
 [1] compilecache(::String) at ./loading.jl:710
 [2] _require(::Symbol) at ./loading.jl:497
 [3] require(::Symbol) at ./loading.jl:405
BenLauwens commented 6 years ago

Checkout branch Python-generators-v0.6 and it will work on Julia v0.6

cortner commented 6 years ago

fantastic - thanks. The other problem I had last time I tried your package was performance related. But I wasn't sure whether it was my fault or a limitation of this package. If I can produce a MWE then I'll just file another issue.

garrison commented 6 years ago

I found inspiration in the new Julia iteration protocol that behaves identically: values are always calculated one ahead.

I don't think this is correct. The new iteration protocol always calls step just before testing isnull. The expected output of my code would then be

yielding 1
1
yielding 2
2
yielding 3
3
yielding 4
4

under the new protocol.

BenLauwens commented 6 years ago

You're right. Now it should do things in the correct order (Julia v0.6) and also the issue when the return type is specified, is solved. Now it should do the same as the revised iteration protocol. So please test;) and keep me informed of problems! Thanks

Ben

garrison commented 6 years ago

Wow that's awesome! :-) Thank you.

Here is a related idea/suggestion: perhaps the macro should error if there is a return statement that returns something. In other words, if a user says return val, we already know that val is going to be ignored, so the friendliest thing is to error. (A bare return statement, on the other hand, is fine.) This will help the user if they accidentally return instead of @yielding something. It's also possible to be more liberal about this later if a need arises, but enforcing this at a later date would mean breaking existing code once again, so I think if you like this idea it makes sense to require it now.

BenLauwens commented 6 years ago

I use the return val in SimJulia to pass a final value as a process terminates. So letting the macro error is not an option. What I can do is generating an error when a value different from nothing is returned when a @resumable function is used as an Iterator but this will be during runtime... What do you think?

garrison commented 6 years ago

I use the return val in SimJulia to pass a final value as a process terminates.

Is there an example of this in action somewhere? Is the intended effect any different from @yield(val); return, which is arguably much more clear and explicit?

garrison commented 6 years ago

(Actually, I can't find a single example of a file that contains both @yield and return statements in SimJulia master.)

BenLauwens commented 6 years ago

There was (is) a use case but as you have mentioned we can always do @yield val; return. I have to check with my researchers using ResumableFunctions if I can do this without breaking their code. I keep you informed (remind me in two weeks if you have had no response;) Kind regards Ben

garrison commented 6 years ago

Sounds good! Another possibility would be to have a deprecation warning instead of an error.

garrison commented 6 years ago

Bump

BenLauwens commented 6 years ago

@garrison

I have implemented your request in branch Python-generators-v0.6 Please checkout. A warning will be generated. If OK I will do ASAP a release for Julia v0.6.

If I get no negative feedback, I will make it an error. This will be the default behaviour for Julia v0.7. Kind regards

Ben

garrison commented 6 years ago

As far as I can tell, this now behaves precisely how I would like it to. One (minor) remaining thing:

(val == :nothing || val == nothing) && return expr

I would suggest changing this line to

val == :nothing && return expr

This way, a return statement will work without warning, but an explicit return nothing will warn. Since it is not possible to return a value, I think the plain return statement is most appropriate style. (This is, of course, my humble opinion.)

Kind regards, Jim