JuliaAI / JuliaAI/MLJModelInterface.jl

mlj_model macro creates methods with bad line number info

Open
#23 2 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Julia
Stars
39
Forks
8
PR merge metrics
No merged PRs in 30d

Description

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.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by tracing the @mlj_model macro's constructor-building path and inspect the generated CodeInfo and linetable shown in the report. Compare the proposed :push_loc and :pop_loc meta-expression approach with the linked SimpleTraits.jl demo PR. Done means generated methods attribute their locations to the model-interface source rather than :none or Julia's logging.jl.

Written by the indexing model from the issue text.

Assessment

Tech stack
julia
Domain
tooling
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.