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

mlj_model macro creates methods with bad line number info #23

Open
timholy opened this issue Mar 4, 2020 · 2 comments
Open

mlj_model macro creates methods with bad line number info #23

timholy opened this issue Mar 4, 2020 · 2 comments

Comments

@timholy
Copy link

timholy commented Mar 4, 2020

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 timholy/Revise.jl#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 mauro3/SimpleTraits.jl#6, although note that was for a rather old version of Julia.

@ablaom
Copy link
Member

ablaom commented Mar 4, 2020

Many thanks for the expert feedback.

@timholy
Copy link
Author

timholy commented Mar 4, 2020

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

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

No branches or pull requests

2 participants