Skip to content
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

WIP Generalize Fix1 and Fix2 #36180

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

goretkin
Copy link
Contributor

@goretkin goretkin commented Jun 7, 2020

I have wanted a generalized version of Fix1 and Fix2 that allows binding any pattern of positional arguments. I think something like the implementation in this PR should be a drop-in replacement.

But I'm having trouble trying it out because I am getting an error during compilation

[...]
multidimensional.jl
permuteddimsarray.jl
broadcast.jl
missing.jl
version.jl
error during bootstrap:

jl_method_error_bare at /home/ggoretkin/repos/julia/src/gf.c:1842
jl_method_error at /home/ggoretkin/repos/julia/src/gf.c:1860
jl_lookup_generic_ at /home/ggoretkin/repos/julia/src/gf.c:2384 [inlined]
jl_apply_generic at /home/ggoretkin/repos/julia/src/gf.c:2405
in at ./strings/search.jl:141
∉ at ./operators.jl:1123
show_type_name at ./show.jl:567
show_datatype at ./show.jl:620
show at ./show.jl:499
_jl_invoke at /home/ggoretkin/repos/julia/src/gf.c:2242 [inlined]
jl_apply_generic at /home/ggoretkin/repos/julia/src/gf.c:2409
_show_default at ./show.jl:395
show_default at ./show.jl:391 [inlined]
show at ./show.jl:386
print at ./strings/io.jl:35
unknown function (ip: 0x7f8ba8cb9642)
_jl_invoke at /home/ggoretkin/repos/julia/src/gf.c:2242 [inlined]
jl_apply_generic at /home/ggoretkin/repos/julia/src/gf.c:2409
print_to_string at ./strings/io.jl:135
string at ./strings/io.jl:174
unknown function (ip: 0x7f8ba8cb9412)
_jl_invoke at /home/ggoretkin/repos/julia/src/gf.c:2242 [inlined]
jl_apply_generic at /home/ggoretkin/repos/julia/src/gf.c:2409
jl_apply at /home/ggoretkin/repos/julia/src/julia.h:1701 [inlined]
do_call at /home/ggoretkin/repos/julia/src/interpreter.c:117
eval_value at /home/ggoretkin/repos/julia/src/interpreter.c:206
eval_stmt_value at /home/ggoretkin/repos/julia/src/interpreter.c:157 [inlined]
eval_body at /home/ggoretkin/repos/julia/src/interpreter.c:566
jl_interpret_toplevel_thunk at /home/ggoretkin/repos/julia/src/interpreter.c:660
top-level scope at version.jl:236
jl_toplevel_eval_flex at /home/ggoretkin/repos/julia/src/toplevel.c:840
jl_parse_eval_all at /home/ggoretkin/repos/julia/src/toplevel.c:957
jl_load_ at /home/ggoretkin/repos/julia/src/toplevel.c:1004
include at ./boot.jl:329 [inlined]
include at ./Base.jl:15
include at ./Base.jl:17
_jl_invoke at /home/ggoretkin/repos/julia/src/gf.c:2225 [inlined]
jl_apply_generic at /home/ggoretkin/repos/julia/src/gf.c:2409
jl_apply at /home/ggoretkin/repos/julia/src/julia.h:1701 [inlined]
do_call at /home/ggoretkin/repos/julia/src/interpreter.c:117
eval_value at /home/ggoretkin/repos/julia/src/interpreter.c:206
eval_stmt_value at /home/ggoretkin/repos/julia/src/interpreter.c:157 [inlined]
eval_body at /home/ggoretkin/repos/julia/src/interpreter.c:566
jl_interpret_toplevel_thunk at /home/ggoretkin/repos/julia/src/interpreter.c:660
top-level scope at Base.jl:206
jl_toplevel_eval_flex at /home/ggoretkin/repos/julia/src/toplevel.c:840
jl_eval_module_expr at /home/ggoretkin/repos/julia/src/toplevel.c:197
jl_toplevel_eval_flex at /home/ggoretkin/repos/julia/src/toplevel.c:666
jl_parse_eval_all at /home/ggoretkin/repos/julia/src/toplevel.c:957
jl_load_ at /home/ggoretkin/repos/julia/src/toplevel.c:1004
include at ./boot.jl:329
_jl_invoke at /home/ggoretkin/repos/julia/src/gf.c:2242 [inlined]
jl_apply_generic at /home/ggoretkin/repos/julia/src/gf.c:2409
jl_apply at /home/ggoretkin/repos/julia/src/julia.h:1701 [inlined]
do_call at /home/ggoretkin/repos/julia/src/interpreter.c:117
eval_value at /home/ggoretkin/repos/julia/src/interpreter.c:206
eval_stmt_value at /home/ggoretkin/repos/julia/src/interpreter.c:157 [inlined]
eval_body at /home/ggoretkin/repos/julia/src/interpreter.c:566
jl_interpret_toplevel_thunk at /home/ggoretkin/repos/julia/src/interpreter.c:660
top-level scope at sysimg.jl:3
jl_toplevel_eval_flex at /home/ggoretkin/repos/julia/src/toplevel.c:840
jl_parse_eval_all at /home/ggoretkin/repos/julia/src/toplevel.c:957
jl_load_ at /home/ggoretkin/repos/julia/src/toplevel.c:1004
jl_load at /home/ggoretkin/repos/julia/src/toplevel.c:1017
exec_program at /home/ggoretkin/repos/julia/ui/repl.c:45
true_main at /home/ggoretkin/repos/julia/ui/repl.c:118
main at /home/ggoretkin/repos/julia/ui/repl.c:227
__libc_start_main at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
_start at /home/ggoretkin/repos/julia/usr/bin/julia (unknown line)

I tried allowing pred.x by defining the getproperty method, but I got a different error during compilation: "cannot add methods to a builtin function". Perhaps getproperty during bootstrapping and getproperty in the runtime are different. That is fine, since the occurances of e.g. pred.x in Base can be fixed, and the getproperty method can be used for a deprecation cycle.

base/operators.jl Outdated Show resolved Hide resolved
Co-authored-by: Takafumi Arakaki <takafumi.a@gmail.com>
@tkf
Copy link
Member

tkf commented Jun 8, 2020

BTW, Fix1 and Fix2 are added mainly because it is useful for dispatch, IIUC. So, unless there is a need for dispatching to partially evaluated functions returned from Base/stdlib, I don't think there is a very strong positive reason to add this in Base.

(But the implementation itself LGTM so maybe it's nice to share this as a package.)

@goretkin
Copy link
Contributor Author

goretkin commented Jun 8, 2020

@tkf thanks for the comments. I agree that the dispatch is the most important part of this (otherwise you can just use anonymous functions). I attempted to include motivation in this issue: #36181 . I think the discussion around the merits of generalizing (or not) Fix1 and Fix2 should happen there.

@tkf
Copy link
Member

tkf commented Jun 8, 2020

OK, I commented in the issue.

@andyferris
Copy link
Member

I think even if functions in Base don’t need e.g. a Fix3 it would still be useful to have definitions that 3rd party packages could use and to allow for blind composition.

@JeffBezanson
Copy link
Sponsor Member

This makes me nervous (mostly in terms of compiler performance), since it uses something very general and complex to implement something very simple and low-level. I don't have an easy answer though.

@goretkin
Copy link
Contributor Author

since it uses something very general and complex to implement something very simple and low-level

I think I understand the nervousness but I don't understand the details. This might not help me get a better sense, but I'll try to ask a question.

If there were an abstract type AbstractFix, then Fix1 and Fix2 and Bind could subtype it. Suppose dispatch happened based on a trait interface to AbstractFix, so instead of f(::Fix2{typeof(f), T}, args...), it would be f_traits(::typeof(f), ::Tuple{Nothing, Some{T}}, args...)

Then you could avoid the structural recursion in interleave by keeping the current implementation of (f::Fix2)(y) = f.f(y, f.x)

Then would that be more or less general/complex/low-level/simple?

@kolia
Copy link

kolia commented Nov 24, 2020

This would also be very useful for long-term serialization of objects containing partial function applications. Anonymous functions get temporary symbols, which break if you deserialize from a context where the owning module has fewer/more anonymous functions defined than at serialization time. So your deserialization can fail even if all you did was add a completely unrelated function that contains an anonymous function definition to the package.

This happens for JLD2, which is for long term storage, and obviously also for julia serialization, which is not.

With this PR, you can replace enclosed anonymous functions with Binds and long-term serialization will work as long as the referenced named functions are still there (and mean the same thing) in the deserialization context.

One domain where you're likely to want to long-term-serialize objects is machine learning, where you've trained a model for a long time and want to use it months later; models often contain anonymous data reshaping/wrangling functions.

@goretkin
Copy link
Contributor Author

@kolia there is registered package https://github.com/goretkin/FixArgs.jl . It would be great if you could see if it is suitable for (de-)serialization.

@kolia
Copy link

kolia commented Nov 25, 2020

@kolia there is registered package https://github.com/goretkin/FixArgs.jl . It would be great if you could see if it is suitable for (de-)serialization.

Yup, both Base.Fix{1,2} and the generalization you've defined in FixArgs.jl work well for long-term serialization, because the type representing the partial function is a regular struct, instead of a DataType with a gensymmed name as is the case for anonymous functions.

The main issue for long-term serialization is the gensymming, because the gensymmed names are all that julia's serializer and JLD2 store, and that is fragile; adding unrelated code to the module often changes existing gensymmed names.

[It doesn't have to be this way though, a serializer could be implemented that serializes anonymous functions themselves instead of just references to their names. BSON.jl tries to do this, but it's imperfect and has other issues.]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants