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

Can't use @load inside a package #613

Closed
CameronBieganek opened this issue Jul 22, 2020 · 25 comments
Closed

Can't use @load inside a package #613

CameronBieganek opened this issue Jul 22, 2020 · 25 comments

Comments

@CameronBieganek
Copy link

Reproducible Example

Create a package as follows.

Package structure

TestMLJ/
├── Manifest.toml
├── Project.toml
└── src
    └── TestMLJ.jl

Project.toml

name = "TestMLJ"
uuid = "4191ff91-1a08-48a9-a49b-84d00a67d6cb"
version = "0.1.0"

[deps]
DecisionTree = "7806a523-6efd-50cb-b5f6-3fa6f1930dbb"
MLJ = "add582a8-e3ab-11e8-2d5e-e98b27df1bc7"
MLJModels = "d491faf4-2d78-11e9-2867-c94bc002c0b7"

TestMLJ.jl

module TestMLJ

using MLJ

@load RandomForestClassifier pkg=DecisionTree

end

Project environment

(TestMLJ) pkg> status
Project TestMLJ v0.1.0
Status `~/projects/TestMLJ/Project.toml`
  [7806a523] DecisionTree v0.10.7
  [add582a8] MLJ v0.12.0
  [d491faf4] MLJModels v0.11.2

Error

julia> using TestMLJ
[ Info: Precompiling TestMLJ [4191ff91-1a08-48a9-a49b-84d00a67d6cb]
WARNING: could not import MLJModels.DecisionTree_ into TestMLJ
ERROR: LoadError: LoadError: UndefVarError: DecisionTree_ not defined
Stacktrace:
 [1] getproperty(::Module, ::Symbol) at ./Base.jl:26
 [2] top-level scope at /Users/bieganek/.julia/packages/MLJModels/BQAzu/src/loading.jl:124
 [3] eval at ./boot.jl:331 [inlined]
 [4] eval(::Expr) at /Users/bieganek/projects/TestMLJ/src/TestMLJ.jl:1
 [5] load(::NamedTuple{(:name, :package_name, :is_supervised, :docstring, :hyperparameter_ranges, :hyperparameter_types, :hyperparameters, :implemented_methods, :is_pure_julia, :is_wrapper, :load_path, :package_license, :package_url, :package_uuid, :prediction_type, :supports_online, :supports_weights, :input_scitype, :target_scitype, :output_scitype),Tuple{String,String,Bool,String,NTuple{8,Nothing},NTuple{8,String},NTuple{8,Symbol},Array{Symbol,1},Bool,Bool,String,String,String,String,Symbol,Bool,Bool,UnionAll,UnionAll,DataType}}; modl::Module, verbosity::Int64, name::Nothing) at /Users/bieganek/.julia/packages/MLJModels/BQAzu/src/loading.jl:128
 [6] load(::String; pkg::String, kwargs::Base.Iterators.Pairs{Symbol,Any,Tuple{Symbol,Symbol},NamedTuple{(:modl, :verbosity),Tuple{Module,Int64}}}) at /Users/bieganek/.julia/packages/MLJModels/BQAzu/src/loading.jl:135
 [7] @load(::LineNumberNode, ::Module, ::Any, ::Vararg{Any,N} where N) at /Users/bieganek/.julia/packages/MLJModels/BQAzu/src/loading.jl:184
 [8] include(::Module, ::String) at ./Base.jl:377
 [9] top-level scope at none:2
 [10] eval at ./boot.jl:331 [inlined]
 [11] eval(::Expr) at ./client.jl:449
 [12] top-level scope at ./none:3
in expression starting at /Users/bieganek/projects/TestMLJ/src/TestMLJ.jl:5
in expression starting at /Users/bieganek/projects/TestMLJ/src/TestMLJ.jl:5
ERROR: Failed to precompile TestMLJ [4191ff91-1a08-48a9-a49b-84d00a67d6cb] to /Users/bieganek/.julia/compiled/v1.4/TestMLJ/JCWpN_nW22B.ji.
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] compilecache(::Base.PkgId, ::String) at ./loading.jl:1272
 [3] _require(::Base.PkgId) at ./loading.jl:1029
 [4] require(::Base.PkgId) at ./loading.jl:927
 [5] require(::Module, ::Symbol) at ./loading.jl:922
@ablaom
Copy link
Member

ablaom commented Jul 22, 2020

Thanks for reporting!

This is not expected. Can you instead do

 load("RandomForestClassifier", pkg="DecisionTree", modl=TestMLJ)

and report any output (this version is verbose by default)?

@CameronBieganek
Copy link
Author

Sure, the output with load("RandomForestClassifier", ...) is this:

julia> using TestMLJ
[ Info: Precompiling TestMLJ [4191ff91-1a08-48a9-a49b-84d00a67d6cb]
[ Info: Loading into module "TestMLJ": 
import MLJModels ✔
import DecisionTree ✔
import MLJModels.DecisionTree_ WARNING: could not import MLJModels.DecisionTree_ into TestMLJ
ERROR: LoadError: UndefVarError: DecisionTree_ not defined
Stacktrace:
 [1] getproperty(::Module, ::Symbol) at ./Base.jl:26
 [2] top-level scope at /Users/bieganek/.julia/packages/MLJModels/BQAzu/src/loading.jl:124
 [3] eval at ./boot.jl:331 [inlined]
 [4] eval(::Expr) at /Users/bieganek/projects/TestMLJ/src/TestMLJ.jl:1
 [5] load(::NamedTuple{(:name, :package_name, :is_supervised, :docstring, :hyperparameter_ranges, :hyperparameter_types, :hyperparameters, :implemented_methods, :is_pure_julia, :is_wrapper, :load_path, :package_license, :package_url, :package_uuid, :prediction_type, :supports_online, :supports_weights, :input_scitype, :target_scitype, :output_scitype),Tuple{String,String,Bool,String,NTuple{8,Nothing},NTuple{8,String},NTuple{8,Symbol},Array{Symbol,1},Bool,Bool,String,String,String,String,Symbol,Bool,Bool,UnionAll,UnionAll,DataType}}; modl::Module, verbosity::Int64, name::Nothing) at /Users/bieganek/.julia/packages/MLJModels/BQAzu/src/loading.jl:128
 [6] load(::String; pkg::String, kwargs::Base.Iterators.Pairs{Symbol,Module,Tuple{Symbol},NamedTuple{(:modl,),Tuple{Module}}}) at /Users/bieganek/.julia/packages/MLJModels/BQAzu/src/loading.jl:135
 [7] top-level scope at /Users/bieganek/projects/TestMLJ/src/TestMLJ.jl:6
 [8] include(::Module, ::String) at ./Base.jl:377
 [9] top-level scope at none:2
 [10] eval at ./boot.jl:331 [inlined]
 [11] eval(::Expr) at ./client.jl:449
 [12] top-level scope at ./none:3
in expression starting at /Users/bieganek/projects/TestMLJ/src/TestMLJ.jl:6
ERROR: Failed to precompile TestMLJ [4191ff91-1a08-48a9-a49b-84d00a67d6cb] to /Users/bieganek/.julia/compiled/v1.4/TestMLJ/JCWpN_nW22B.ji.
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] compilecache(::Base.PkgId, ::String) at ./loading.jl:1272
 [3] _require(::Base.PkgId) at ./loading.jl:1029
 [4] require(::Base.PkgId) at ./loading.jl:927
 [5] require(::Module, ::Symbol) at ./loading.jl:922

@CameronBieganek
Copy link
Author

Oddly enough, the following does not reproduce the error:

julia> module A
           using MLJ
           @load RandomForestClassifier pkg=DecisionTree
       end
Main.A

julia> using .A

@CameronBieganek
Copy link
Author

CameronBieganek commented Jul 22, 2020

EDIT: Added import DecisionTree and I still get the same error.

Having the following in the TestMLJ.jl file also produces a similar error:

module TestMLJ

using MLJ
import DecisionTree
import MLJModels

const RandomForestClassifier = MLJModels.DecisionTree_.RandomForestClassifier

end
julia> using TestMLJ
[ Info: Precompiling TestMLJ [4191ff91-1a08-48a9-a49b-84d00a67d6cb]
ERROR: LoadError: UndefVarError: DecisionTree_ not defined
Stacktrace:
 [1] getproperty(::Module, ::Symbol) at ./Base.jl:26
 [2] top-level scope at /Users/bieganek/projects/TestMLJ/src/TestMLJ.jl:8
 [3] include(::Module, ::String) at ./Base.jl:377
 [4] top-level scope at none:2
 [5] eval at ./boot.jl:331 [inlined]
 [6] eval(::Expr) at ./client.jl:449
 [7] top-level scope at ./none:3
in expression starting at /Users/bieganek/projects/TestMLJ/src/TestMLJ.jl:8
ERROR: Failed to precompile TestMLJ [4191ff91-1a08-48a9-a49b-84d00a67d6cb] to /Users/bieganek/.julia/compiled/v1.4/TestMLJ/JCWpN_nW22B.ji.
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] compilecache(::Base.PkgId, ::String) at ./loading.jl:1272
 [3] _require(::Base.PkgId) at ./loading.jl:1029
 [4] require(::Base.PkgId) at ./loading.jl:927
 [5] require(::Module, ::Symbol) at ./loading.jl:922

@CameronBieganek
Copy link
Author

Do you happen to know a temporary workaround? I wanted to package up my code so that I could run it on the university cluster.

@ablaom
Copy link
Member

ablaom commented Jul 23, 2020

Okay, this is a known issue which I suspect is somehow related to Requires. JuliaAI/MLJModels.jl#22 .

Is it just the one package DecisionTree you want to use or are there are bunch of model-providing packages you want?

@ablaom
Copy link
Member

ablaom commented Jul 23, 2020

One way to confirm my suspicion is for you to load something not provided by MLJModels. Can you try @load ImageClassifier instead?

I'll try to get you a workaround by the end of my day.

@ablaom
Copy link
Member

ablaom commented Jul 23, 2020

Yes, it's a Requires issue, it seems to me. I can load ImageClassifier (not provided by MLJModels) fine.

@CameronBieganek
Copy link
Author

Is it just the one package DecisionTree you want to use or are there are bunch of model-providing packages you want?

Yeah, my code also uses XGBoost and LIBSVM (and MLJLinearModels).

Not on my work machine right now. I'll double check tomorrow that I can do @load ImageClassifier.

@ablaom
Copy link
Member

ablaom commented Jul 23, 2020

Requires is not working as expected unless you put the loading code in the __init__ of your module - a partial workaround. (You have to wrap use of RandomForestClassifier in functions, you won't be able to easily use macros like @pipeline, and so forth). Sorry, that's the best I can do for now. There is a long term fix as described in issue linked above.

TestMLJ.jl

module TestMLJ

using MLJ

function do_stuff()
    model = RandomForestClassifier()
    X, y = @load_iris;
    e = evaluate(model, X, y, measure=cross_entropy)
    return e
end

function __init__()
    eval(quote
         load("RandomForestClassifier",
              pkg="DecisionTree",
              modl=TestMLJ)
         end)
end

end module

Then using TestMLJ; do_stuff() works for me.

@CameronBieganek
Copy link
Author

Thanks for the workaround! Unfortunately, I do use @pipeline, so the workaround doesn't quite work for me. Now I'm trying a modified version of the original workaround from @rssdev10. This is what I have in the TestMLJ.jl file:

module TestMLJ

using MLJ
import DecisionTree

import LIBSVM
import XGBoost
import MLJModels

include(joinpath(MLJModels.srcdir, "DecisionTree.jl"))
using .DecisionTree_: RandomForestClassifier

include(joinpath(MLJModels.srcdir, "XGBoost.jl"))
using .XGBoost_: XGBoostClassifier

include(joinpath(MLJModels.srcdir, "LIBSVM.jl"))
using .LIBSVM_: LinearSVC, SVC

end

Note that this also requires adding some dependencies to the Project.toml file. Statistics, Tables, and MLJModelInterface have to be added since they're used in different places in the DecisionTree_, XGBoost_, and LIBSVM_ modules. So the Project.toml file for TestMLJ looks like this:

name = "TestMLJ"
uuid = "4191ff91-1a08-48a9-a49b-84d00a67d6cb"
version = "0.1.0"

[deps]
DecisionTree = "7806a523-6efd-50cb-b5f6-3fa6f1930dbb"
LIBSVM = "b1bec4e5-fd48-53fe-b0cb-9723c09d164b"
MLJ = "add582a8-e3ab-11e8-2d5e-e98b27df1bc7"
MLJLinearModels = "6ee0df7b-362f-4a72-a706-9e79364fb692"
MLJModelInterface = "e80e1ace-859a-464e-9ed9-23947d8ae3ea"
MLJModels = "d491faf4-2d78-11e9-2867-c94bc002c0b7"
Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2"
Tables = "bd369af6-aec1-5ad0-b16a-f7cc5008161c"
XGBoost = "009559a3-9522-5dbb-924b-0b6ed2b22bb9"

However, sometimes (although not always) when I do using TestMLJ I get some warnings that are a little worrying:

julia> using TestMLJ
[ Info: Precompiling TestMLJ [4191ff91-1a08-48a9-a49b-84d00a67d6cb]
WARNING: Method definition (::Type{TestMLJ.DecisionTree_.DecisionTreeClassifier})() in module DecisionTree_ overwritten.
  ** incremental compilation may be fatally broken for this module **

WARNING: Method definition Type##kw(Any, Type{TestMLJ.DecisionTree_.DecisionTreeClassifier}) in module DecisionTree_ overwritten.
  ** incremental compilation may be fatally broken for this module **

WARNING: Method definition (::Type{TestMLJ.DecisionTree_.RandomForestClassifier})() in module DecisionTree_ overwritten.
  ** incremental compilation may be fatally broken for this module **

WARNING: Method definition Type##kw(Any, Type{TestMLJ.DecisionTree_.RandomForestClassifier}) in module DecisionTree_ overwritten.
  ** incremental compilation may be fatally broken for this module **

WARNING: Method definition (::Type{TestMLJ.DecisionTree_.AdaBoostStumpClassifier})() in module DecisionTree_ overwritten.
  ** incremental compilation may be fatally broken for this module **

WARNING: Method definition Type##kw(Any, Type{TestMLJ.DecisionTree_.AdaBoostStumpClassifier}) in module DecisionTree_ overwritten.
  ** incremental compilation may be fatally broken for this module **

WARNING: Method definition (::Type{TestMLJ.DecisionTree_.DecisionTreeRegressor})() in module DecisionTree_ overwritten.
  ** incremental compilation may be fatally broken for this module **

WARNING: Method definition Type##kw(Any, Type{TestMLJ.DecisionTree_.DecisionTreeRegressor}) in module DecisionTree_ overwritten.
  ** incremental compilation may be fatally broken for this module **

WARNING: Method definition (::Type{TestMLJ.DecisionTree_.RandomForestRegressor})() in module DecisionTree_ overwritten.
  ** incremental compilation may be fatally broken for this module **

WARNING: Method definition Type##kw(Any, Type{TestMLJ.DecisionTree_.RandomForestRegressor}) in module DecisionTree_ overwritten.
  ** incremental compilation may be fatally broken for this module **

@OkonSamuel
Copy link
Member

OkonSamuel commented Jul 23, 2020

Did you by any chance redefine any of the above methods.
These warnings pop up in julia whenever it forced to recompile an already compile method or Type.
Could you try again this time commenting out import DecisionTree

@CameronBieganek
Copy link
Author

Did you by any chance redefine any of the above methods.

No, this is just the warning messages for TestMLJ. The TestMLJ module shown above is a MWE with no methods defined.

Could you try again this time commenting out import DecisionTree

When I try that I get this error message:

julia> using TestMLJ
[ Info: Precompiling TestMLJ [4191ff91-1a08-48a9-a49b-84d00a67d6cb]
WARNING: could not import TestMLJ.DecisionTree into DecisionTree_
ERROR: LoadError: LoadError: UndefVarError: DecisionTree not defined
Stacktrace:
 [1] top-level scope at /Users/bieganek/.julia/packages/MLJModels/BQAzu/src/DecisionTree.jl:12
 [2] include(::Module, ::String) at ./Base.jl:377
 [3] include(::String) at /Users/bieganek/projects/TestMLJ/src/TestMLJ.jl:1
 [4] top-level scope at /Users/bieganek/projects/TestMLJ/src/TestMLJ.jl:15
 [5] include(::Module, ::String) at ./Base.jl:377
 [6] top-level scope at none:2
 [7] eval at ./boot.jl:331 [inlined]
 [8] eval(::Expr) at ./client.jl:449
 [9] top-level scope at ./none:3
in expression starting at /Users/bieganek/.julia/packages/MLJModels/BQAzu/src/DecisionTree.jl:12
in expression starting at /Users/bieganek/projects/TestMLJ/src/TestMLJ.jl:15
ERROR: Failed to precompile TestMLJ [4191ff91-1a08-48a9-a49b-84d00a67d6cb] to /Users/bieganek/.julia/compiled/v1.4/TestMLJ/JCWpN_nW22B.ji.
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] compilecache(::Base.PkgId, ::String) at ./loading.jl:1272
 [3] _require(::Base.PkgId) at ./loading.jl:1029
 [4] require(::Base.PkgId) at ./loading.jl:927
 [5] require(::Module, ::Symbol) at ./loading.jl:922

The import or using for DecisionTree appears to be necessary since the DecisionTree_ module has a line like this:

import ..DecisionTree

As I'm guessing you've noticed, the various types in those warning messages are defined in DecisionTree.jl, but for the ScikitLearn.jl interface, not the MLJ interface. Obviously the sorts of code loading shenanigans that I have in TestMLJ are not ideal. 😂 Hopefully the various package authors will natively implement the MLJ interface soon.

@OkonSamuel
Copy link
Member

Yeah import ..DecisionTree is the culprit

@OkonSamuel
Copy link
Member

I think i may have a solution.

module TestMLJ

using MLJ
import MLJModels
import DecisionTree
import LIBSVM
import XGBoost
#import MLJModels

#include(joinpath(MLJModels.srcdir, "DecisionTree.jl"))

using MLJModels.DecisionTree_: RandomForestClassifier

include(joinpath(MLJModels.srcdir, "XGBoost.jl"))
using .XGBoost_: XGBoostClassifier

include(joinpath(MLJModels.srcdir, "LIBSVM.jl"))
using .LIBSVM_: LinearSVC, SVC

end # module

@CameronBieganek
Copy link
Author

Hmm, that doesn't seem to work for me. I get this:

julia> using TestMLJ
[ Info: Precompiling TestMLJ [4191ff91-1a08-48a9-a49b-84d00a67d6cb]
ERROR: LoadError: UndefVarError: DecisionTree_ not defined
Stacktrace:
 [1] include(::Module, ::String) at ./Base.jl:377
 [2] top-level scope at none:2
 [3] eval at ./boot.jl:331 [inlined]
 [4] eval(::Expr) at ./client.jl:449
 [5] top-level scope at ./none:3
in expression starting at /Users/bieganek/projects/TestMLJ/src/TestMLJ.jl:12
ERROR: Failed to precompile TestMLJ [4191ff91-1a08-48a9-a49b-84d00a67d6cb] to /Users/bieganek/.julia/compiled/v1.4/TestMLJ/JCWpN_nW22B.ji.
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] compilecache(::Base.PkgId, ::String) at ./loading.jl:1272
 [3] _require(::Base.PkgId) at ./loading.jl:1029
 [4] require(::Base.PkgId) at ./loading.jl:927
 [5] require(::Module, ::Symbol) at ./loading.jl:922

@CameronBieganek
Copy link
Author

CameronBieganek commented Jul 23, 2020

It seems like the cause of those warnings might actually be the metadata_pkg and metadata_model functions that are used in the DecisionTree.jl file. Note that those functions are not used in the XGBoost.jl or LIBSVM.jl files.

Both of those functions have a line like this that might be causing problems:

parentmodule(T).eval(ex)

The reason I noticed this is because I made a fork of MLJModels where I got rid of @requires and made DecisionTree.jl, XGBoost.jl, and LIBSVM.jl explicit dependencies. However, I still get those method redefinition warnings.

EDIT: Maybe not. I commented out that part of the code and still get the warnings. 😂

@OkonSamuel
Copy link
Member

OkonSamuel commented Jul 23, 2020

@CameronBieganek Hurray. I have finally found what's causing the warnings.

(c::TreePrinter)(depth) = DT.print_tree(c.tree, depth)
(c::TreePrinter)() = DT.print_tree(c.tree, 5)

The above methods were somehow being redefined due to a bug @mlj_model macro in model definition at MLJModels.jl.
@mlj_model macro is defined in MLJModelInterface.jl so i forked the repo with and fixed the macro. here

Here is the module definition i used

module TestMLJ

using MLJ
import DecisionTree

import LIBSVM
import XGBoost
import MLJModels

include(joinpath(MLJModels.srcdir, "DecisionTree.jl"))
using .DecisionTree_: RandomForestClassifier

include(joinpath(MLJModels.srcdir, "XGBoost.jl"))
using .XGBoost_: XGBoostClassifier

include(joinpath(MLJModels.srcdir, "LIBSVM.jl"))
using .LIBSVM_: LinearSVC, SVC

end

Tested it 3 times already an i haven't seen any Warnings

@ablaom
Copy link
Member

ablaom commented Jul 23, 2020

Thanks @OkonSamuel for jumping in here.

Just so it is clear. I can confirm the original issue is only an issue with packages whose glue code is lazy loaded from MLJModels. The issue will ultimately be resolved with the planned "�disintegration" of MLJModels. See here for the progress on this: JuliaAI/MLJModels.jl#244 (comment)

@CameronBieganek
Copy link
Author

@OkonSamuel Awesome, thanks for looking into those warnings!

@ablaom Yeah, the warnings that @OkonSamuel and I were discussing were a bit tangential to the issue in the original post. They just made me a little worried about my workaround. However, my code seems to be working fine despite the warnings, so it's all good. 😃

@OkonSamuel
Copy link
Member

@CameronBieganek hope the warning are gone now? if so can we close this?

@CameronBieganek
Copy link
Author

CameronBieganek commented Jul 24, 2020

Well, the original issue of not being able to load models inside a package is not completely resolved, though we do have a workaround. It might be worth keeping this issue open so that other people who encounter the same problem can find this issue. Although the issue title might not be conducive to discoverability...

Of course there's also JuliaAI/MLJModels.jl#22 tracking this, so it's up to you guys.

EDIT: Yes, the warnings are gone now. :)

@CameronBieganek CameronBieganek changed the title LoadError: UndefVarError: DecisionTree_ not defined Can't use @load inside a package Jul 24, 2020
@CameronBieganek
Copy link
Author

Ok, I changed the name to make it more discoverable, although now it just looks like a duplicate of #321. 😂

@OkonSamuel
Copy link
Member

Thanks @CameronBieganek . I think i'll just let @ablaom decide here

@ablaom
Copy link
Member

ablaom commented Jul 26, 2020

Closing in favour of #321.

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

3 participants