diff --git a/src/cache.jl b/src/cache.jl
index 4b6e19a3b0570d7ef7bc8faa9c210ca91f9d238f..2511c807270b5cfe952013a604fd1f33725466f3 100644
--- a/src/cache.jl
+++ b/src/cache.jl
@@ -215,24 +215,28 @@ See: `@with_signature`.
function broadcast_volume!(f::Function, D::T_Dispatch, C::Cache) where T_Dispatch
has_signature(f, D) || error("broadcast_volume!: Function '$f' has no signature!")
accepts_syms, returns_syms = accepts(f, D), returns(f, D)
+ returns_tell_success = tells_success(f, D)
n_accepts, n_returns = length(accepts_syms), length(returns_syms)
accepts_vars = NTuple{n_accepts,Vector{Float64}}( get_variable(C, a) for a in accepts_syms )
returns_vars = NTuple{n_returns,Vector{Float64}}( get_variable(C, r) for r in returns_syms )
f_args = args -> f(args, D)
- _broadcast_volume!(f_args, C, accepts_vars, returns_vars)
- return
+ success = _broadcast_volume!(f_args, C, accepts_vars, returns_vars, returns_tell_success)
+ return success
end
function _broadcast_volume!(f, C::Cache,
- accepts_vars::NTuple{N,Vector{Float64}},
- returns_vars::NTuple{M,Vector{Float64}}) where {N,M}
+ accepts_vars::NTuple{N,Vector{Float64}},
+ returns_vars::NTuple{M,Vector{Float64}},
+ returns_tell_success::Bool) where {N,M}
soa_accepts = StructArray{NTuple{N,Float64}}( accepts_vars )
soa_returns = StructArray{NTuple{M,Float64}}( returns_vars )
npts = length(first(accepts_vars))
for idx = 1:npts
- soa_returns[idx] = f(soa_accepts[idx])
+ success, result = f(soa_returns[idx])
+ returns_tell_success && !success && return false
+ soa_returns[idx] = result
end
- return
+ return true
end
diff --git a/src/projects/GRHD/cons2prim_implementations.jl b/src/projects/GRHD/cons2prim.jl
similarity index 78%
rename from src/projects/GRHD/cons2prim_implementations.jl
rename to src/projects/GRHD/cons2prim.jl
index b7a1d332f5a1fa1443b9ae45e665aab55f08234c..d068dc37721ddbd4670ecfc87b26d8096bc2a443 100644
--- a/src/projects/GRHD/cons2prim_implementations.jl
+++ b/src/projects/GRHD/cons2prim.jl
@@ -1,3 +1,14 @@
+@with_signature function cons2prim(equation::Equation)
+ @accepts D, Sr, tau, p, r, B
+ S2 = (Sr/B)^2
+ p = find_pressure_insane(D, S2, tau, p, r, equation)
+ rho = Formulae.Ï(B, D, Sr, tau, p)
+ vr = Formulae.vr(B, D, Sr, tau, p)
+ eps = Formulae.ϵ(B, D, Sr, tau, p)
+ @returns rho, vr, eps, p
+end
+
+
#######################################################################
# Insane version - bail out if anything is wrong #
#######################################################################
@@ -27,8 +38,7 @@ function pressure_equation_insane(tau, p, D, S2, eos, p0)
end
-@with_signature function find_pressure_insane(equation::Equation)
- @accepts D, S2, tau, p, r, B
+function find_pressure_insane(D, S2, tau, p, r, equation::Equation)
@unpack eos = equation
# upper limit on particle velocity
@@ -53,5 +63,5 @@ end
p = max(pmin, p0)
# println(pmin)
- @returns p
+ return p
end
diff --git a/src/projects/GRHD/equation.jl b/src/projects/GRHD/equation.jl
index 415b15a6b8ae5879b1a2ae1366e805338af767aa..65e93bfb4ba035591290df3d13afbffef1379712 100644
--- a/src/projects/GRHD/equation.jl
+++ b/src/projects/GRHD/equation.jl
@@ -41,6 +41,9 @@ end
#######################################################################
+include("cons2prim.jl")
+
+
module Formulae
### Auxilary functions
@@ -175,20 +178,6 @@ end
end
-include("cons2prim_implementations.jl")
-
-
-@with_signature function cons2prim(equation::Equation)
- @accepts D, Sr, tau, p, r, B
- S2 = (Sr/B)^2
- p, = find_pressure_insane((D, S2, tau, p, r, B), equation)
- rho = Formulae.Ï(B, D, Sr, tau, p)
- vr = Formulae.vr(B, D, Sr, tau, p)
- eps = Formulae.ϵ(B, D, Sr, tau, p)
- @returns rho, vr, eps, p
-end
-
-
@with_signature function sources(equation::Equation)
@accepts rho, vr, p, eps, r, A, A_r, B, B_r
src_sD = Formulae.sD(r, A, A_r, B, B_r, rho, vr, eps, p)
diff --git a/src/with_signature.jl b/src/with_signature.jl
index 8408c3d88403c2efc508d1d0dd52ebad1233ba04..998236b3b19398248964e670e638598bb3f5831c 100644
--- a/src/with_signature.jl
+++ b/src/with_signature.jl
@@ -1,5 +1,5 @@
export @with_signature, @accepts, @returns,
- has_signature, accepts, returns, signature, state_indices
+ has_signature, accepts, returns, signature, state_indices, tells_success, success
"""
@@ -43,8 +43,9 @@ macro accepts(ex); ex; end
macro returns(ex); ex; end
-_is_state(ex) = ex isa Expr && ex.head === :call && ex.args[1] === :State
-_is_tuple(ex) = ex isa Expr && ex.head === :tuple
+_is_state(ex) = ex isa Expr && ex.head === :call && ex.args[1] === :State
+_is_success(ex) = ex isa Expr && ex.head === :call && ex.args[1] === :Success
+_is_tuple(ex) = ex isa Expr && ex.head === :tuple
# gather list of all variables and a list of indices of variables
@@ -82,14 +83,56 @@ function accepts(ex)
end
+# extract and return the symbols of the provided return variables
+# the second return value is a boolean indicating whether the first element in the
+# variable list indicates success of the call
+# Examples:
+#
+# returns(:(Success(c), a, b, d))
+# ->
+# (:c, :a, :b, :d), true
+#
+# returns(:(a, b, d))
+# ->
+# (:a, :b, :d), false
function returns(ex)
- if ex isa Symbol
- (ex,)
+ all, tells_success = if ex isa Symbol
+ (ex,), false
+ elseif _is_success(ex)
+ vars = ex.args[2:end]
+ length(vars) != 1 && error("@returns: 'Success' can only have one argument")
+ v = vars[1]
+ !(typeof(v) in [Symbol, Bool]) && error(
+ "@returns: 'Success' arguments must be a Symbol or a Boolean")
+ (v,), true
elseif _is_tuple(ex)
- tuple(ex.args...)
+ vars = ex.args[1:end]
+ all, tells_success = Union{Bool,Symbol}[], false
+ if _is_success(vars[1])
+ v = vars[1].args[2:end]
+ length(v) != 1 && error("@returns: 'Success' can only have one argument")
+ v_arg = v[1]
+ !(typeof(v_arg) in [Symbol, Bool]) && error(
+ "@returns: 'Success' arguments must be a Symbol or a Boolean")
+ push!(all, v_arg)
+ tells_success = true
+ vars = ex.args[2:end]
+ end
+ last_i = length(ex.args) - length(vars) + 1
+ for v in vars
+ if _is_success(v)
+ error("@returns: 'Success' can only appear as the first return value")
+ elseif v isa Symbol
+ push!(all, v)
+ else
+ error("@returns: expected a variable name or a Success tag")
+ end
+ end
+ all, tells_success
else
- error("@returns: expected '@returns f_a, f_b, ...'")
+ error("@returns: expected '@returns [Success(value)] f_a, f_b, ...'")
end
+ tuple(all...), tells_success
end
@@ -128,10 +171,17 @@ function with_signature(ex)
# gather variables from place holders
accepts_variables, pos_state_variables = accepts(accepts_variables)
- returns_variables = returns(returns_variables)
+ returns_variables, returns_tells_success = returns(returns_variables)
+ # replace macros in function body
fn_body[1] = Expr(:(=), Expr(:tuple, accepts_variables...), :args)
- fn_body[end] = Expr(:tuple, returns_variables...)
+ fn_body[end] = if returns_tells_success
+ success_var, returns_variables = returns_variables[1], returns_variables[2:end]
+ Expr(:tuple, :(Bool($success_var)), Expr(:tuple, returns_variables...))
+ else
+ # assume success by default
+ Expr(:tuple, :(true), Expr(:tuple, returns_variables...))
+ end
# sanitize function arguments
(first_arg isa Expr && first_arg.head === :(::)) || error(
@@ -149,9 +199,10 @@ function with_signature(ex)
new_ex = esc(quote
$ex
- $(thismod).signature(::typeof($(fn_name)), ::$(dispatch_type)) =
+ $(thismod).signature(::typeof($(fn_name)), ::$(dispatch_type)) =
($(accepts_variables), $(returns_variables))
$(thismod).state_indices(::typeof($(fn_name)), ::$(dispatch_type)) = $(pos_state_variables)
+ $(thismod).tells_success(::typeof($(fn_name)), ::$(dispatch_type)) = $(returns_tells_success)
$(thismod).has_signature(::typeof($(fn_name)), ::$(dispatch_type)) = true
end)
@@ -171,5 +222,7 @@ signature(fn, dispatch) = error(
"function '$fn' with dispatch '$(dispatch)' was not defined using @with_signature")
state_indices(fn, dispatch) = error(
"function '$fn' with dispatch '$(dispatch)' was not defined using @with_signature")
-accepts(fn, dispatch) = signature(fn, dispatch)[1]
-returns(fn, dispatch) = signature(fn, dispatch)[2]
+tells_success(fn, dispatch) = error(
+ "function '$fn' with dispatch '$(dispatch)' was not defined using @with_signature")
+accepts(fn, dispatch) = signature(fn, dispatch)[1]
+returns(fn, dispatch) = signature(fn, dispatch)[2]
diff --git a/test/dg1dTests/src/README_TESTS_FAIL.md b/test/dg1dTests/src/README_TESTS_FAIL.md
new file mode 100644
index 0000000000000000000000000000000000000000..13c8e7ede34f64c01be2fce12d11b325ed044580
--- /dev/null
+++ b/test/dg1dTests/src/README_TESTS_FAIL.md
@@ -0,0 +1,39 @@
+I implemented the Success tag for return arguments for the @with_signature macro.
+The @with_signature tests already pass.
+The issue now is that Riemann solver tests (actually just calls) don't pass, because
+we adding the success return value now messes things up with broadcasting and argument
+destruction.
+
+E.g. lax_friedrich_flux likes to call speeds(...) for both sides,
+ vl, vr = speed(args_lhs, equation), speed(args_rhs, equation)
+ fl, fr = flux(args_lhs, equation), flux(args_rhs, equation)
+The correct version of the above would be
+ (success_vl, vl), (success_vr, vr) = speed(args_lhs, equation), speed(args_rhs, equation)
+ (success_fl, fl), (success_fr, fr) = flux(args_lhs, equation), flux(args_rhs, equation)
+which makes it quite verbose and inconvinient.
+Imagine nested multiple @with_signature calls would at some point add also overhead for
+the success checks.
+Not sure if we can live with this by saying that if you call the functions you should always
+be prepared for unexpected behavior and thus check the success. So far we never did and
+we seemingly did not have problems.
+
+A more simpler approach to the problem would be to check for validity of results produced
+by a call to a signature function after the whole computation is done.
+E.g. if `compute_something` outputs results into `u, v` then after the call we can check
+those arrays.
+Assuming everything goes well and success is true and `u,v` are ok and the arrays are of length N.
+Checking the returned success value for each point will involve N if checks, provided
+the returned success value indicates validity for both of `u, v`.
+The first to fail would immediately stop the loop and one would be able to start with the
+recovering strategy. Not sure what performance implications this extra return value has.
+If we were to only check validity after the broadcasted call finished, we will then have
+to do N checks for each variable, so in this case 2 N. Furthermore, we would have to wait
+for the whole broadcasting event to finish before we can then check and only then start
+with the rescue mission. However, I would assume that performance of the loop can only
+be better in this case since we have one return value less to care about and we don't have to
+do if checks in the loop.
+
+A thrid approach would be to use a global as a failure indicator that can be set from within
+every function and later be checked after the loop is done (or perhaps also within the loop).
+This way we don't have to deal with multiple return values and the validity check after the
+loop is simple. The drawback is that we introduce coupling.
diff --git a/test/dg1dTests/src/test_with_signature.jl b/test/dg1dTests/src/test_with_signature.jl
index 0fe286441dea94354e06ba6ac9e24964a2b2d620..f865e3b074a2568ea55416b18e40397e264f14b8 100644
--- a/test/dg1dTests/src/test_with_signature.jl
+++ b/test/dg1dTests/src/test_with_signature.jl
@@ -12,10 +12,12 @@
@test accepts(fn1, dummyeq) == (:u1, :u2)
@test returns(fn1, dummyeq) == (:rhs_u1, :rhs_u2)
@test state_indices(fn1, dummyeq) == ()
+ @test tells_success(fn1, dummyeq) == false
@test has_signature(fn1, dummyeq) == true
args = (1.0, 2.0)
- obtained = fn1(args, dummyeq)
+ success, obtained = fn1(args, dummyeq)
expected = (4.0, 5.0)
+ @test success == true
@test obtained == expected
# @accepts and @returns with only one argument each
@@ -27,10 +29,12 @@
@test accepts(fn2, dummyeq) == (:u1,)
@test returns(fn2, dummyeq) == (:rhs_u1,)
@test state_indices(fn2, dummyeq) == ()
+ @test tells_success(fn2, dummyeq) == false
@test has_signature(fn2, dummyeq) == true
args = (1.0,)
- obtained = fn2(args, dummyeq)
+ success, obtained = fn2(args, dummyeq)
expected = (2.0,)
+ @test success == true
@test obtained == expected
@@ -43,10 +47,12 @@
@test accepts(fn3, dummyeq) == (:u1, :u2)
@test returns(fn3, dummyeq) == (:rhs_u1, :rhs_u2)
@test state_indices(fn3, dummyeq) == (1,2)
+ @test tells_success(fn3, dummyeq) == false
@test has_signature(fn3, dummyeq) == true
args = (1.0, 2.0)
- obtained = fn3(args, dummyeq)
+ success, obtained = fn3(args, dummyeq)
expected = (4.0, 5.0)
+ @test success == true
@test obtained == expected
# @accepts single State variable
@@ -58,10 +64,12 @@
@test accepts(fn4, dummyeq) == (:u1,)
@test returns(fn4, dummyeq) == (:rhs_u1,)
@test state_indices(fn4, dummyeq) == (1,)
+ @test tells_success(fn4, dummyeq) == false
@test has_signature(fn4, dummyeq) == true
args = (1.0,)
- obtained = fn4(args, dummyeq)
+ success, obtained = fn4(args, dummyeq)
expected = (2.0,)
+ @test success == true
@test obtained == expected
# @accepts mix of variables and State variables
@@ -73,10 +81,12 @@
@test accepts(fn5, dummyeq) == (:u1,:u2,:u3,:u4)
@test returns(fn5, dummyeq) == (:rhs_u1,)
@test state_indices(fn5, dummyeq) == (1,3,4)
+ @test tells_success(fn5, dummyeq) == false
@test has_signature(fn5, dummyeq) == true
args = (1.0,2.0,3.0,4.0)
- obtained = fn5(args, dummyeq)
+ success, obtained = fn5(args, dummyeq)
expected = (11.0,)
+ @test success == true
@test obtained == expected
# no variable name for dispatch type required
@@ -88,12 +98,50 @@
@test accepts(fn6, dummyeq) == (:u1,)
@test returns(fn6, dummyeq) == (:rhs_u1,)
@test state_indices(fn6, dummyeq) == ()
+ @test tells_success(fn6, dummyeq) == false
@test has_signature(fn6, dummyeq) == true
args = (1.0,)
- obtained = fn6(args, dummyeq)
+ success, obtained = fn6(args, dummyeq)
expected = (2.0,)
+ @test success == true
@test obtained == expected
+ # @returns success
+ @with_signature function fn7(eq::DummyEquation)
+ @accepts u1, u2, u3, u4
+ rhs_u1 = 2.0 * u1 + u2 + u3 + u4
+ @returns Success(true), rhs_u1
+ end
+ @test accepts(fn7, dummyeq) == (:u1,:u2,:u3,:u4)
+ @test returns(fn7, dummyeq) == (:rhs_u1,)
+ @test state_indices(fn7, dummyeq) == ()
+ @test tells_success(fn7, dummyeq) == true
+ @test has_signature(fn7, dummyeq) == true
+ args = (1.0,2.0,3.0,4.0)
+ success, obtained = fn7(args, dummyeq)
+ expected = (11.0,)
+ @test success == true
+ @test obtained == expected
+
+ # @returns success
+ @with_signature function fn8(eq::DummyEquation)
+ @accepts u1, u2, u3, u4
+ rhs_u1 = 2.0 * u1 + u2 + u3 + u4
+ success = false
+ @returns Success(success), rhs_u1
+ end
+ @test accepts(fn8, dummyeq) == (:u1,:u2,:u3,:u4)
+ @test returns(fn8, dummyeq) == (:rhs_u1,)
+ @test state_indices(fn8, dummyeq) == ()
+ @test tells_success(fn8, dummyeq) == true
+ @test has_signature(fn8, dummyeq) == true
+ args = (1.0,2.0,3.0,4.0)
+ success, obtained = fn8(args, dummyeq)
+ expected = (11.0,)
+ @test success == false
+ @test obtained == expected
+
+
# function with unused variable rhs_u2 in body throws normal UndefVarError
@with_signature function fn_undefvar(eq::DummyEquation)
@accepts u1
@@ -174,7 +222,22 @@
@test_throws ErrorException accepts(fn_with_no_signature,dummyeq)
@test_throws ErrorException returns(fn_with_no_signature,dummyeq)
@test_throws ErrorException state_indices(fn_with_no_signature,dummyeq)
+ @test_throws ErrorException tells_success(fn_with_no_signature,dummyeq)
# expect the trait function is just false
has_signature(fn_with_no_signature, dummyeq) == false
+ # Success must be first return argument if provided
+ @test_throws LoadError @eval @with_signature function fn_success_not_first(eq::DummyEquation)
+ @accepts u1, u2
+ rhs_u1, rhs_u2 = 2.0 * u1, 2.0 * u2
+ @returns rhs_u1, Success(false), rhs_u2
+ end
+
+ # pure Booleans as first return arguments does not work either
+ @test_throws LoadError @eval @with_signature function fn_success_not_first(eq::DummyEquation)
+ @accepts u1, u2
+ rhs_u1, rhs_u2 = 2.0 * u1, 2.0 * u2
+ @returns false, rhs_u1, rhs_u2
+ end
+
end