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

0.2: refactoring #28

Merged
merged 12 commits into from
Nov 19, 2019
Merged

0.2: refactoring #28

merged 12 commits into from
Nov 19, 2019

Conversation

thautwarm
Copy link
Member

Solved issues:

@codecov
Copy link

codecov bot commented Nov 17, 2019

Codecov Report

Merging #28 into master will increase coverage by 10.81%.
The diff coverage is 87.01%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master      #28       +/-   ##
===========================================
+ Coverage   75.31%   86.13%   +10.81%     
===========================================
  Files           6        8        +2     
  Lines         239      274       +35     
===========================================
+ Hits          180      236       +56     
+ Misses         59       38       -21
Impacted Files Coverage Δ
src/ngg/ngg.jl 100% <100%> (ø)
src/ngg/runtime_fns.jl 96.87% <100%> (ø)
src/ngg/typeable.jl 68.96% <57.14%> (ø)
src/lens.jl 71.42% <71.42%> (ø)
src/func_arg_decs.jl 82.85% <82.85%> (ø)
src/GeneralizedGenerated.jl 85.71% <83.33%> (+2.38%) ⬆️
src/closure_conv.jl 91.66% <91.54%> (+9.05%) ⬆️
... and 1 more

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 9815ca0...393ccdf. Read the comment docs.

@thautwarm
Copy link
Member Author

Hi Chad, besides removing a lot of bugs, specifying the evaluation module becomes feasible.

julia> module S
           run(y) = y + 1
       end
Main.S

julia> @gg m function g(m::Module, y) :(run(y)) end
# the global variable `run` is from the module `m`
g (generic function with 1 method)

julia> g(S, 1)
2

julia> struct S
           run :: Function
       end
Main.S

julia> @gg m function g(m::S, y) :(run(y)) end
# the global variable `run` is from the datum `m`
g (generic function with 1 method)

julia> g(S(x -> x + 1), 1)
2

@thautwarm
Copy link
Member Author

@cscherrer

@cscherrer
Copy link
Collaborator

Wow, this looks great! I'll be able to try it out soon, and I'll let you know how it goes.

Thank you!!

@cscherrer
Copy link
Collaborator

Something's throwing me about this, I think it's that we usually (in functions, etc) first bind a value to a name, and then use that name to use the value in some context. In

@gg m function g(m::Module, y) :(run(y)) end

it seems the binding occurs in the sending reference to m, while the first reference specifies how the bound value (some module) is used. Is that right?

@thautwarm
Copy link
Member Author

Yes, @gg m fn means the bound value m is treated as the evaluation module of global variables in fn!

@thautwarm
Copy link
Member Author

I'm to add more tests tomorrow and make a minor release.

@cscherrer
Copy link
Collaborator

cscherrer commented Nov 17, 2019

This is working great from Main within the Soss environment, but is somehow failing within Weave.jl. Am I doing this right?

@inline function rand(m::JointDistribution)
    @show getmodule(m.model) # just for debugging
    return _rand(getmodule(m.model), m.model, m.args)
end

@gg M function _rand(M::Module, _m::Model, _args) 
    type2model(_m) |> sourceRand() |> loadvals(_args, NamedTuple())
end

Weaving my .jmd file produces things like

julia> truth = rand(m(X=X));
getmodule(m.model) = Main.WeaveSandBox1
Error: UndefVarError: M not defined

Looks like rand works until it calls _rand

Here's the first part of the stack trace:

  Test threw exception
  Expression: buildREADME()
  UndefVarError: M not defined
  Stacktrace:
   [1] capture_output(::Expr, ::Module, ::Bool, ::Bool, ::Bool, ::Bool) at /home/chad/.julia/packages/Weave/UOxmI/src/run.jl:241
   [2] run_code(::Weave.CodeChunk, ::Weave.Report, ::Module) at /home/chad/.julia/packages/Weave/UOxmI/src/run.jl:208
   [3] eval_chunk(::Weave.CodeChunk, ::Weave.Report, ::Module) at /home/chad/.julia/packages/Weave/UOxmI/src/run.jl:289
   [4] run_chunk(::Weave.CodeChunk, ::Weave.WeaveDoc, ::Weave.Report, ::Module) at /home/chad/.julia/packages/Weave/UOxmI/src/run.jl:130
   [5] #run#34(::String, ::Symbol, ::Symbol, ::Dict{Symbol,Int64}, ::String, ::Nothing, ::String, ::Symbol, ::Bool, ::typeof(run), ::Weave.WeaveDoc) at /home/chad/.julia/packages/Weave/UOxmI/src/run.jl:94
   [6] (::Base.var"#kw##run")(::NamedTuple{(:doctype, :mod, :out_path, :args, :fig_path, :fig_ext, :cache_path, :cache, :throw_errors),Tuple{String,Symbol,Symbol,Dict{Symbol,Int64},String,Nothing,String,Symbol,Bool}}, ::typeof(run), ::Weave.WeaveDoc) at ./none:0
   [7] #weave#16(::String, ::Symbol, ::Symbol, ::Dict{Symbol,Int64}, ::Symbol, ::String, ::Nothing, ::String, ::Symbol, ::Bool, ::Nothing, ::Nothing, ::Nothing, ::Array{String,1}, ::String, ::typeof(weave), ::String) at /home/chad/.julia/packages/Weave/UOxmI/src/Weave.jl:121
   [8] #weave at ./none:0 [inlined]
   [9] buildREADME() at /home/chad/git/jl/Soss/test/runtests.jl:6
   [10] top-level scope at /home/chad/git/jl/Soss/test/runtests.jl:12
   [11] top-level scope at /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.3/Test/src/Test.jl:1107
   [12] top-level scope at /home/chad/git/jl/Soss/test/runtests.jl:12
  caused by [exception 1]
  UndefVarError: M not defined
  Stacktrace:
   [1] (::GeneralizedGenerated.NGG.RuntimeFn{TypeEncoding(list(β, x)),TypeEncoding(nil(GeneralizedGenerated.NGG.Argument)),TypeEncoding(begin
      M.Normal(M.:*(x', β), 1)
  end),:function})(::Array{Float64,1}, ::SubArray{Float64,1,Array{Float64,2},Tuple{Int64,Base.Slice{Base.OneTo{Int64}}},true}) at /home/chad/.julia/packages/GeneralizedGenerated/AROoR/src/ngg/runtime_fns.jl:70
   [2] #_#20 at /home/chad/.julia/packages/GeneralizedGenerated/AROoR/src/closure.jl:6 [inlined]
   [3] Closure at /home/chad/.julia/packages/GeneralizedGenerated/AROoR/src/closure.jl:6 [inlined]
   [4] For(::GeneralizedGenerated.Closure{function = (β, x;) -> begin
      M.Normal(M.:*(x', β), 1)
  end,Tuple{Array{Float64,1}}}, ::Base.Generator{Base.OneTo{Int64},Base.var"#174#175"{Array{Float64,2}}}) at /home/chad/git/jl/Soss/src/distributions/for.jl:77
   [5] macro expansion at /home/chad/.julia/packages/GeneralizedGenerated/AROoR/src/closure_conv.jl:48 [inlined]
   [6] _rand(::Module, ::Model{NamedTuple{(:X,),T} where T<:Tuple,TypeEncoding(begin
      β ~ Normal() |> iid(size(X, 2))
      y ~ For(eachrow(X)) do x
              Normal(x' * β, 1)
          end
  end),TypeEncoding(Main.WeaveSandBox0)}, ::NamedTuple{(:X,),Tuple{Array{Float64,2}}}) at /home/chad/.julia/packages/GeneralizedGenerated/AROoR/src/closure_conv.jl:109
   [7] rand(::Soss.JointDistribution{NamedTuple{(:X,),Tuple{Array{Float64,2}}},NamedTuple{(:X,),T} where T<:Tuple,TypeEncoding(begin
      β ~ Normal() |> iid(size(X, 2))
      y ~ For(eachrow(X)) do x
              Normal(x' * β, 1)
          end
  end),TypeEncoding(Main.WeaveSandBox0)}) at /home/chad/git/jl/Soss/src/primitives/rand.jl:9

cscherrer added a commit to cscherrer/Soss.jl that referenced this pull request Nov 17, 2019
@ChrisRackauckas
Copy link
Contributor

ChrisRackauckas commented Nov 17, 2019

Could we either try to get this or #30 through fast please? The current release is broken and no upper bound on GeneralizedGenerated is able to fix it. Please ping me when you have a hotfix to tag and I'll fast-track it through the registry since this ScopedVar issue has been very downstream breaking.

@thautwarm
Copy link
Member Author

@ChrisRackauckas Okay, in fact the problem is caused by the minor release of dependencies. You can specify the version of JuliaVariables.jl as 0.1.x.

@thautwarm
Copy link
Member Author

@ChrisRackauckas Could you change this to 0.1.3? There're several bug fixes from 0.1.0 to 0.1.3? #30 (comment)

allow making the module as a freevar for nested functions,
if the module is specified in the form of an argument.
@thautwarm
Copy link
Member Author

@cscherrer Hi Chad, fixed now!
Only if you're specifying the module with an argument, there'll be a runtime performance hit.

julia> using Soss


julia> m = @model X begin
           β ~ Normal() |> iid(size(X,2))
           y ~ For(eachrow(X)) do x
               Normal(x' * β, 1)
           end
       end;

julia> import Random; Random.seed!(3);
julia> X = randn(6,2)
6×2 Array{Float64,2}:
  1.19156    0.100793  
 -2.51973   -0.00197414
  2.07481    1.00879   
 -0.97325    0.844223  
 -0.101607   1.15807   
 -1.54251   -0.475159 
julia> truth = rand(m(X=X));
getmodule(m.model) = Main

julia> pairs(truth)
pairs(::NamedTuple) with 3 entries:
  :X => [1.19156 0.100793; -2.51973 -0.00197414;  ; -0.101607 1.15807; -1.54251 -0.475159]
   => [0.0718727, -0.51281]
  :y => [0.100793, -2.51973, 2.07481, 0.844223, 1.15807, -0.475159]

@cscherrer
Copy link
Collaborator

Do I need a different version of the dependencies?

(Soss) pkg> add GeneralizedGenerated#0.2
  Updating registry at `~/.julia/registries/General`
  Updating git-repo `https://github.com/JuliaRegistries/General.git`
  Updating git-repo `https://github.com/thautwarm/GeneralizedGenerated.jl.git`
 Resolving package versions...
ERROR: Could not parse project /home/chad/.julia/packages/GeneralizedGenerated/bANQN/Project.toml: CompositeException(Any[Pkg.TOML.ParserError(461, 475, "duplicate key `JuliaVariables`")])

@thautwarm
Copy link
Member Author

Do I need a different version of the dependencies?

No, currently there's a stupid problem, I've just fixed it and let's wait for the CI.

@cscherrer
Copy link
Collaborator

It's getting there! 0.2 now seems to mostly work. But when I ]test in Soss, I get

(Soss) pkg> test
   Testing Soss
 Resolving package versions...
[ Info: Weaving chunk 1 from line 14
[ Info: Weaving chunk 2 from line 35
[ Info: Weaving chunk 3 from line 40
[ Info: Weaving chunk 4 from line 45
[ Info: Weaving chunk 5 from line 49
[ Info: Weaving chunk 6 from line 56
README: Error During Test at /home/chad/git/jl/Soss/test/runtests.jl:12
  Test threw exception
  Expression: buildREADME()
  UndefVarError: as not defined

as is from TransformVariables.jl, which is a dependency of Soss. It's strange that...

  • This works just fine from the command line
  • It also works if I using TransformVariables: as in the jmd file. (But then other the same problem happens with SymPy.

I guess this must be something to do with weave running in a different module. As I type this, I wonder if I may need to re-export everything I import in Soss. Trying that now...

@thautwarm
Copy link
Member Author

@ChrisRackauckas I've tested ModelingToolKit with this banch, all went well.

@thautwarm
Copy link
Member Author

@cscherrer Chad, please check Slack direct message and I've given your 2 solutions.

@ChrisRackauckas
Copy link
Contributor

sounds good!

@cscherrer cscherrer merged commit 83cf2f4 into master Nov 19, 2019
cscherrer added a commit to cscherrer/Soss.jl that referenced this pull request Nov 28, 2019
* update dependencies

* remove junk files

* remove junk files

* Reorganizing (Good idea Kusti)

* internals.md

* update README

* ignore .vscode directory

* Revert "ignore .vscode directory"

This reverts commit 5753f58.

* Delete settings.json

* split jointdistribution.jl

* parameterize model by module

* make `runtests` throw errors

* Parameterize Model by module

* debugging

* Update for JuliaStaging/GeneralizedGenerated.jl#28

* insert `$sympy`(SymPy) & `$as`(TransformVariables.jl); (#63)

tests passed

* Package upper bounds

* fix demo

* Being less fancy with logpdf

* Move logpdf(m,x,::typeof(codegen)) to codegen.jl

* Fixing stuff I just broke

* working on markovblanket

* working on Markov Blanket

* markov blanket

* add bodyVariables

* export bodyVariables

* make `toposort` us `arguments` instead of `freeVariables`

* up

* ixnay on the ype piracytay

* Type infer for module param (#66)

* apply changes

* fix #65: might need more changes

* Update dependencies + README

* update README

* update README
cscherrer added a commit to cscherrer/Soss.jl that referenced this pull request Dec 10, 2019
* update dependencies
* Reorganize repo into folders
* Parameterize Model by module
* Update for JuliaStaging/GeneralizedGenerated.jl#28
* Add package upper bounds
* new `markovBlanket` combinator
* Improved symbolic simplification and codegen
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.

None yet

3 participants