From 6beea29b612705ec927fdae294fed1df58f02e63 Mon Sep 17 00:00:00 2001 From: Joshua Lampert Date: Tue, 6 Aug 2024 00:26:54 +0200 Subject: [PATCH 1/2] add bang to set_sqrt_type and set_log_type --- src/auxiliary/math.jl | 50 +++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/src/auxiliary/math.jl b/src/auxiliary/math.jl index 0bd5ad438f3..1edfe57549e 100644 --- a/src/auxiliary/math.jl +++ b/src/auxiliary/math.jl @@ -23,44 +23,46 @@ function set_polyester!(toggle::Bool; force = true) end """ - Trixi.set_sqrt_type(type; force = true) + Trixi.set_sqrt_type!(type; force = true) Set the `type` of the square root function to be used in Trixi.jl. The default is `"sqrt_Trixi_NaN"` which returns `NaN` for negative arguments instead of throwing an error. -Alternatively, you can set `type` to `"sqrt_Base"` to use the Julia built-in `sqrt` function +Alternatively, you can set `type` to `"sqrt_Base"` to use the Julia built-in `sqrt` function which provides a stack-trace of the error which might come in handy when debugging code. """ -function set_sqrt_type(type; force = true) +function set_sqrt_type!(type; force = true) @assert type == "sqrt_Trixi_NaN"||type == "sqrt_Base" "Only allowed `sqrt` function types are `\"sqrt_Trixi_NaN\"` and `\"sqrt_Base\"`" set_preferences!(TRIXI_UUID, "sqrt" => type, force = force) @info "Please restart Julia and reload Trixi.jl for the `sqrt` computation change to take effect" end +@deprecate set_sqrt_type(type; force = true) set_sqrt_type!(type; force = true) false + @static if _PREFERENCE_SQRT == "sqrt_Trixi_NaN" """ Trixi.sqrt(x::Real) Custom square root function which returns `NaN` for negative arguments instead of throwing an error. - This is required to ensure [correct results for multithreaded computations](https://github.com/trixi-framework/Trixi.jl/issues/1766) - when using the [`Polyester` package](https://github.com/JuliaSIMD/Polyester.jl), + This is required to ensure [correct results for multithreaded computations](https://github.com/trixi-framework/Trixi.jl/issues/1766) + when using the [`Polyester` package](https://github.com/JuliaSIMD/Polyester.jl), i.e., using the `@batch` macro instead of the Julia built-in `@threads` macro, see [`@threaded`](@ref). - We dispatch this function for `Float64, Float32, Float16` to the LLVM intrinsics - `llvm.sqrt.f64`, `llvm.sqrt.f32`, `llvm.sqrt.f16` as for these the LLVM functions can be used out-of the box, + We dispatch this function for `Float64, Float32, Float16` to the LLVM intrinsics + `llvm.sqrt.f64`, `llvm.sqrt.f32`, `llvm.sqrt.f16` as for these the LLVM functions can be used out-of the box, i.e., they return `NaN` for negative arguments. - In principle, one could also use the `sqrt_llvm` call, but for transparency and consistency with [`log`](@ref) we - spell out the datatype-dependent functions here. + In principle, one could also use the `sqrt_llvm` call, but for transparency and consistency with [`log`](@ref) we + spell out the datatype-dependent functions here. For other types, such as integers or dual numbers required for algorithmic differentiation, we fall back to the Julia built-in `sqrt` function after a check for negative arguments. - Since these cases are not performance critical, the check for negativity does not hurt here + Since these cases are not performance critical, the check for negativity does not hurt here and can (as of now) even be optimized away by the compiler due to the implementation of `sqrt` in Julia. - When debugging code, it might be useful to change the implementation of this function to redirect to - the Julia built-in `sqrt` function, as this reports the exact place in code where the domain is violated + When debugging code, it might be useful to change the implementation of this function to redirect to + the Julia built-in `sqrt` function, as this reports the exact place in code where the domain is violated in the stacktrace. - See also [`Trixi.set_sqrt_type`](@ref). + See also [`Trixi.set_sqrt_type!`](@ref). """ @inline sqrt(x::Real) = x < zero(x) ? oftype(x, NaN) : Base.sqrt(x) @@ -74,41 +76,43 @@ end end """ - Trixi.set_log_type(type; force = true) + Trixi.set_log_type!(type; force = true) Set the `type` of the (natural) `log` function to be used in Trixi.jl. The default is `"sqrt_Trixi_NaN"` which returns `NaN` for negative arguments instead of throwing an error. -Alternatively, you can set `type` to `"sqrt_Base"` to use the Julia built-in `sqrt` function +Alternatively, you can set `type` to `"sqrt_Base"` to use the Julia built-in `sqrt` function which provides a stack-trace of the error which might come in handy when debugging code. """ -function set_log_type(type; force = true) +function set_log_type!(type; force = true) @assert type == "log_Trixi_NaN"||type == "log_Base" "Only allowed log function types are `\"log_Trixi_NaN\"` and `\"log_Base\"`." set_preferences!(TRIXI_UUID, "log" => type, force = force) @info "Please restart Julia and reload Trixi.jl for the `log` computation change to take effect" end +@deprecate set_log_type(type; force = true) set_log_type!(type; force = true) false + @static if _PREFERENCE_LOG == "log_Trixi_NaN" """ Trixi.log(x::Real) Custom natural logarithm function which returns `NaN` for negative arguments instead of throwing an error. - This is required to ensure [correct results for multithreaded computations](https://github.com/trixi-framework/Trixi.jl/issues/1766) - when using the [`Polyester` package](https://github.com/JuliaSIMD/Polyester.jl), + This is required to ensure [correct results for multithreaded computations](https://github.com/trixi-framework/Trixi.jl/issues/1766) + when using the [`Polyester` package](https://github.com/JuliaSIMD/Polyester.jl), i.e., using the `@batch` macro instead of the Julia built-in `@threads` macro, see [`@threaded`](@ref). - We dispatch this function for `Float64, Float32, Float16` to the respective LLVM intrinsics - `llvm.log.f64`, `llvm.log.f32`, `llvm.log.f16` as for this the LLVM functions can be used out-of the box, i.e., + We dispatch this function for `Float64, Float32, Float16` to the respective LLVM intrinsics + `llvm.log.f64`, `llvm.log.f32`, `llvm.log.f16` as for this the LLVM functions can be used out-of the box, i.e., they return `NaN` for negative arguments. For other types, such as integers or dual numbers required for algorithmic differentiation, we fall back to the Julia built-in `log` function after a check for negative arguments. Since these cases are not performance critical, the check for negativity does not hurt here. - When debugging code, it might be useful to change the implementation of this function to redirect to - the Julia built-in `log` function, as this reports the exact place in code where the domain is violated + When debugging code, it might be useful to change the implementation of this function to redirect to + the Julia built-in `log` function, as this reports the exact place in code where the domain is violated in the stacktrace. - See also [`Trixi.set_log_type`](@ref). + See also [`Trixi.set_log_type!`](@ref). """ @inline log(x::Real) = x < zero(x) ? oftype(x, NaN) : Base.log(x) From 0084407b05832d98226cc5aafaedf631c5a93482 Mon Sep 17 00:00:00 2001 From: Joshua Lampert <51029046+JoshuaLampert@users.noreply.github.com> Date: Wed, 7 Aug 2024 10:22:25 +0200 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Hendrik Ranocha --- src/auxiliary/math.jl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/auxiliary/math.jl b/src/auxiliary/math.jl index 1edfe57549e..171c631a843 100644 --- a/src/auxiliary/math.jl +++ b/src/auxiliary/math.jl @@ -37,6 +37,7 @@ function set_sqrt_type!(type; force = true) @info "Please restart Julia and reload Trixi.jl for the `sqrt` computation change to take effect" end +# TODO: deprecation introduced in v0.8 @deprecate set_sqrt_type(type; force = true) set_sqrt_type!(type; force = true) false @static if _PREFERENCE_SQRT == "sqrt_Trixi_NaN" @@ -90,6 +91,7 @@ function set_log_type!(type; force = true) @info "Please restart Julia and reload Trixi.jl for the `log` computation change to take effect" end +# TODO: deprecation introduced in v0.8 @deprecate set_log_type(type; force = true) set_log_type!(type; force = true) false @static if _PREFERENCE_LOG == "log_Trixi_NaN"