Hotfixes after #309 (#351)

#309 was merged a bit too soon, for example `bors` was never run on the final version due to some issues. Before we make a release, we should make sure that it all works properly.

Author: torfjelde

.github/workflows/CI.yml

@@ -16,7 +16,7 @@ jobs:
     strategy:
       matrix:
         version:
-          # - '1.3' # minimum supported version
+          - '1.3' # minimum supported version
           - '1' # current stable version
         os:
           - ubuntu-latest

docs/make.jl

@@ -18,6 +18,9 @@ makedocs(;
         r"(Array{.+,\s?1}|Vector{.+})",
         # Older versions will show "Array{...,2}" instead of "Matrix{...}".
         r"(Array{.+,\s?2}|Matrix{.+})",
+        # Errors from macros sometimes result in `LoadError: LoadError:`
+        # rather than `LoadError:`, depending on Julia version.
+        r"ERROR: (LoadError:\s)+",
     ],
 )
 

src/submodel_macro.jl

@@ -212,17 +212,23 @@ function submodel(prefix_expr, expr, ctx=esc(:__context__))
     if prefix_left !== :prefix
         error("$(prefix_left) is not a valid kwarg")
     end
+
+    # The user expects `@submodel ...` to return the
+    # return-value of the `...`, hence we need to capture
+    # the return-value and handle it correctly.
+    @gensym retval
+
     # `prefix=false` => don't prefix, i.e. do nothing to `ctx`.
     # `prefix=true` => automatically determine prefix.
     # `prefix=...` => use it.
     args_assign = getargs_assignment(expr)
     return if args_assign === nothing
         ctx = prefix_submodel_context(prefix, ctx)
-        # In this case we only want to get the `__varinfo__`.
         quote
-            $(esc(:__varinfo__)) = last(
-                $(DynamicPPL._evaluate!!)($(esc(expr)), $(esc(:__varinfo__)), $(ctx))
+            $retval, $(esc(:__varinfo__)) = $(DynamicPPL._evaluate!!)(
+                $(esc(expr)), $(esc(:__varinfo__)), $(ctx)
             )
+            $retval
         end
     else
         L, R = args_assign
@@ -235,9 +241,10 @@ function submodel(prefix_expr, expr, ctx=esc(:__context__))
             )
         end
         quote
-            $(esc(L)), $(esc(:__varinfo__)) = $(DynamicPPL._evaluate!!)(
+            $retval, $(esc(:__varinfo__)) = $(DynamicPPL._evaluate!!)(
                 $(esc(R)), $(esc(:__varinfo__)), $(ctx)
             )
+            $(esc(L)) = $retval
         end
     end
 end

test/compiler.jl

@@ -570,5 +570,15 @@ end
 
         @model demo() = x ~ Normal()
         retval, svi = DynamicPPL.evaluate!!(demo(), SimpleVarInfo(), SamplingContext())
+
+        # Return-value when using `@submodel`
+        @model inner() = x ~ Normal()
+        # Without assignment.
+        @model outer() = @submodel inner()
+        @test outer()() isa Real
+
+        # With assignment.
+        @model outer() = @submodel x = inner()
+        @test outer()() isa Real
     end
 end

test/runtests.jl

@@ -66,6 +66,9 @@ include("test_util.jl")
                 r"(Array{.+,\s?1}|Vector{.+})",
                 # Older versions will show "Array{...,2}" instead of "Matrix{...}".
                 r"(Array{.+,\s?2}|Matrix{.+})",
+                # Errors from macros sometimes result in `LoadError: LoadError:`
+                # rather than `LoadError:`, depending on Julia version.
+                r"ERROR: (LoadError:\s)+",
             ]
             doctest(DynamicPPL; manual=false, doctestfilters=doctestfilters)
         end