JuliaAI / MLJModelInterface.jl

Lightweight package to interface with MLJ
MIT License
37 stars 8 forks source link

mlj_model macro creates methods with bad line number info #23

Open timholy opened 4 years ago

timholy commented 4 years ago

I am not certain whether this is a Julia bug or something you need to fix here, but I think it's probably something you need to fix here. In any case I thought I should report it. I detected it in https://github.com/timholy/Revise.jl/issues/439 (CC @ablaom), as the error comes from inside this conditional block.

Here's the lowered code that gets created for one of your constructors (RandomForestRegression):

stmt3 = :($(Expr(:method, false, JuliaInterpreter.SSAValue(11), CodeInfo(
     @ none within `RandomForestRegressor#53'
1 ──       Core.NewvarNode(:(level))
│          Core.NewvarNode(:(std_level))
│          Core.NewvarNode(:(group))
│          Core.NewvarNode(:(_module))
│          Core.NewvarNode(:(logger))
│          Core.NewvarNode(:(id))
│          Core.NewvarNode(:(file))
│          Core.NewvarNode(:(line))
│    %9  = RandomForestRegressor
│    %10 = Core.fieldtype(%9, 1)
│    %11 = Base.convert(%10, max_depth)
│    %12 = Core.fieldtype(%9, 2)
│    %13 = Base.convert(%12, min_samples_leaf)
│    %14 = Core.fieldtype(%9, 3)
│    %15 = Base.convert(%14, min_samples_split)
│    %16 = Core.fieldtype(%9, 4)
│    %17 = Base.convert(%16, min_purity_increase)
│    %18 = Core.fieldtype(%9, 5)
│    %19 = Base.convert(%18, n_subfeatures)
│    %20 = Core.fieldtype(%9, 6)
│    %21 = Base.convert(%20, n_trees)
│    %22 = Core.fieldtype(%9, 7)
│    %23 = Base.convert(%22, sampling_fraction)
│    %24 = Core.fieldtype(%9, 8)
│    %25 = Base.convert(%24, pdf_smoothing)
│          model = %new(%9, %11, %13, %15, %17, %19, %21, %23, %25)
│    %27 = Base.getproperty(MLJModels.DecisionTree_.MLJModelInterface, :clean!)
│          message = (%27)(model)
│    %29 = MLJModels.DecisionTree_.isempty(message)
└───       goto #3 if not %29
2 ──       goto #13
    └
     @ logging.jl:305 within `RandomForestRegressor#53'
3 ──       level = Base.CoreLogging.Warn
│    @ logging.jl:306 within `RandomForestRegressor#53'
│          std_level = Base.CoreLogging.convert(Base.CoreLogging.LogLevel, level)
│    @ logging.jl:307 within `RandomForestRegressor#53'
│    %34 = std_level
│    %35 = Base.CoreLogging.getindex(Base.CoreLogging._min_enabled_level)
│    %36 = %34 >= %35
└───       goto #12 if not %36
     @ logging.jl:308 within `RandomForestRegressor#53'
4 ──       group = $(QuoteNode("model_def"))
│    @ logging.jl:309 within `RandomForestRegressor#53'
│          _module = MLJModels.DecisionTree_
│    @ logging.jl:310 within `RandomForestRegressor#53'
│          logger = Base.CoreLogging.current_logger_for_env(std_level, group, _module)
│    @ logging.jl:311 within `RandomForestRegressor#53'
│    %41 = logger === Base.CoreLogging.nothing
│    %42 = !%41
└───       goto #12 if not %42
     @ logging.jl:312 within `RandomForestRegressor#53'
5 ──       id = :MLJModels_DecisionTree__f55bc851
│    @ logging.jl:315 within `RandomForestRegressor#53'
│    %45 = Base.CoreLogging.shouldlog(logger, level, _module, group, id)
└───       goto #12 if not %45
     @ logging.jl:316 within `RandomForestRegressor#53'
6 ──       file = "/home/tim/.julia/packages/MLJModelInterface/lb8aH/src/model_def.jl"
│    @ logging.jl:317 within `RandomForestRegressor#53'
└───       line = 126
     @ logging.jl:318 within `RandomForestRegressor#53'
7 ── %49 = $(Expr(:enter, #10))
     @ logging.jl:319 within `RandomForestRegressor#53'
8 ──       msg = message
│    @ logging.jl:320 within `RandomForestRegressor#53'
│          Base.CoreLogging.handle_message(logger, level, msg, _module, group, id, file, line)
└───       $(Expr(:leave, 1))
9 ──       goto #12
10 ┄       $(Expr(:leave, 1))
11 ─       err = $(Expr(:the_exception))
│    @ logging.jl:322 within `RandomForestRegressor#53'
│          Base.CoreLogging.logging_error(logger, level, _module, group, id, file, line, err)
└───       $(Expr(:pop_exception, :(%49)))
     @ logging.jl:327 within `RandomForestRegressor#53'
12 ┄       Base.CoreLogging.nothing
13 ┄       return model
)))))
 @ 1…53
bodycode.linetable = 
Any[Core.LineInfoNode(Symbol("RandomForestRegressor#53"), :none, 0, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 305, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 306, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 307, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 308, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 309, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 310, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 311, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 312, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 315, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 316, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 317, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 318, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 319, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 320, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 322, 0), Core.LineInfoNode(Symbol("RandomForestRegressor#53"), Symbol("logging.jl"), 327, 0)]

The linetable is particularly relevant. The first line is attributed to file :none and line 0. The rest get the line number correct but attribute it to logging.jl (one of Julia's base/ files).

Going off the names of the calls in @mlj_model (in particular from the fact that it looks like you're calling a function to build the constructor), I suspect this isn't a Julia bug but something you need to handle here. You might be able to use :push_loc and :pop_loc meta expressions. A possible demo PR is https://github.com/mauro3/SimpleTraits.jl/pull/6, although note that was for a rather old version of Julia.

ablaom commented 4 years ago

Many thanks for the expert feedback.

timholy commented 4 years ago

If you don't know about this already, see https://docs.julialang.org/en/latest/devdocs/meta/#