Skip to content

Conversation

rjplevin
Copy link
Collaborator

This is a partial fix for module lookup problem that might suffice for the workshop.
It still won't handle nested modules, but we can deal with that next...

@codecov-io
Copy link

codecov-io commented May 23, 2019

Codecov Report

Merging #496 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #496      +/-   ##
=========================================
+ Coverage   80.78%   80.8%   +0.01%     
=========================================
  Files          26      26              
  Lines        1931    1933       +2     
=========================================
+ Hits         1560    1562       +2     
  Misses        371     371
Impacted Files Coverage Δ
src/core/defs.jl 92.28% <100%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c16b847...1b43132. Read the comment docs.

@corakingdon
Copy link
Collaborator

@rjplevin I just tried this out and still get the same error.
If you want to try what I've been running, checkout the computeSCC branch of my fork of MimiFUND here: https://github.com/ckingdon95/fund/tree/computeSCC

and try the following:

julia> using MimiFUND
julia> MimiFUND.compute_scc(year=2020)
ERROR: UndefVarError: Mimi not defined
Stacktrace:
 [1] Mimi.ComponentInstance{Mimi.ComponentInstanceVariables{NamedTuple{(:output,),Tuple{Mimi.TimestepArray{Mimi.FixedTimestep{1950,1,LAST} where LAST,Union{Missing, Float64},1}}}},Mimi.ComponentInstanceParameters{NamedTuple{(:add, :input),Tuple{Mimi.TimestepArray{Mimi.FixedTimestep{1951,1,LAST} where LAST,Float64,1},Mimi.TimestepArray{Mimi.FixedTimestep{1950,1,LAST} where LAST,Union{Missing, Float64},1}}}}}(::Mimi.ComponentDef, ::Mimi.ComponentInstanceVariables{NamedTuple{(:output,),Tuple{Mimi.TimestepArray{Mimi.FixedTimestep{1950,1,LAST} where LAST,Union{Missing, Float64},1}}}}, ::Mimi.ComponentInstanceParameters{NamedTuple{(:add, :input),Tuple{Mimi.TimestepArray{Mimi.FixedTimestep{1951,1,LAST} where LAST,Float64,1},Mimi.TimestepArray{Mimi.FixedTimestep{1950,1,LAST} where LAST,Union{Missing, Float64},1}}}}, ::Int64, ::Int64, ::Symbol) at C:\Users\kingdon\.julia\dev\Mimi\src\core\types.jl:360
 [2] build(::Mimi.ModelDef) at C:\Users\kingdon\.julia\dev\Mimi\src\core\build.jl:161
 [3] build at C:\Users\kingdon\.julia\dev\Mimi\src\core\build.jl:90 [inlined]
 [4] #run#114(::Int64, ::Nothing, ::Function, ::Mimi.Model) at C:\Users\kingdon\.julia\dev\Mimi\src\core\model.jl:395
 [5] #run at .\none:0 [inlined]
 [6] #run#12 at C:\Users\kingdon\.julia\dev\Mimi\src\core\build.jl:189 [inlined]
 [7] #run at .\none:0 [inlined]
 [8] #_compute_scc#13(::Int64, ::Symbol, ::Int64, ::Bool, ::Float64, ::Float64, ::Function, ::Mimi.MarginalModel) at C:\Users\kingdon\.julia\dev\MimiFUND\src\new_marginaldamages.jl:47
 [9] #_compute_scc at .\none:0 [inlined]
 [10] #compute_scc#11(::Int64, ::Symbol, ::Int64, ::Bool, ::Float64, ::Float64, ::Function, ::Mimi.Model) at C:\Users\kingdon\.julia\dev\MimiFUND\src\new_marginaldamages.jl:21
 [11] #compute_scc at .\none:0 [inlined] (repeats 2 times)
 [12] top-level scope at none:0

Copy link
Collaborator

@corakingdon corakingdon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approve! (pending on the tests passing I guess)

Copy link
Collaborator

@davidanthoff davidanthoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Can we open an issue to try to find a more general fix for this? We don't need to sort this out for the workshop, but it seems that we should be able to solve this in some more general way?

@davidanthoff davidanthoff merged commit 8308f39 into master May 23, 2019
@davidanthoff davidanthoff deleted the fix-module-lookup branch May 23, 2019 17:04
@rjplevin
Copy link
Collaborator Author

Sure. I'll do that now.

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.

4 participants