Skip to content

Commit 8f8579e

Browse files
Fix QA (Aqua/JET) findings and align downgrade compat floor
Builds on the Symbolics-v6 cap to make the QA and Downgrade groups green: Downgrade: raise the package Manifolds compat floor from 0.9 to 0.10 so it matches the test env's Manifolds cap (needed for OptimizationManopt 1.3.2). julia-downgrade-compat was pinning the root floor to Manifolds 0.9.10, which had an empty intersection with the test env's 0.10 -> Downgrade now resolves the minimal set (Manifolds 0.10.0, ...). Aqua: - Drop the unused IfElse and Dictionaries deps (genuine stale deps). - Disambiguate Base.in(::Num/::Symbolic, ::CustomDomain) against Symbolics' in(::Num/::Symbolic, ::Domain) (CustomDomain <: Domain made the two overlap). - Configure run_qa's Aqua via aqua_kwargs: treat the deliberately-extended non-owned functions (log/tr/inv/sqrt/logdet/distance/xlogx and the Symbolics arguments/hasmetadata/promote_symtype hooks) as own for the piracy check, and exclude the 10 atom functions whose scalar+array @register_symbolic registrations generate benign internal ambiguities. All remaining ambiguities are internal to SymbolicAnalysis (no cross-package ambiguity remains). JET (real code bugs surfaced by report_package, run in :typo mode): - lorentz_log_barrier referenced an undefined ; a = (0,...,0,1) makes the Lorentzian inner product reduce to -p[end], so the barrier is -log(-1+p[end]). - lognormcdf called logcdf(Normal, x) (the type) which is a MethodError; use the standard-normal instance logcdf(Normal(), x). - find_curvature/find_gcurvature could fall through with / undefined (the branch did not return its value); return it and default f_curvature/f_monotonicity to the existing 'unknown' semantics. Switch test/qa/qa.jl to SciMLTesting.run_qa (the shared harness, matching ModelingToolkit) with jet=true and mode=:typo. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 8815943 commit 8f8579e

6 files changed

Lines changed: 52 additions & 21 deletions

File tree

Project.toml

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,8 @@ version = "0.3.6"
66
[deps]
77
DSP = "717857b8-e6f2-59f4-9121-6e50c889abd2"
88
DataStructures = "864edb3b-99cc-5e75-8d2d-829cb0a9cfe8"
9-
Dictionaries = "85a47980-9c8c-11e8-2b9f-f7ca1fa99fb4"
109
Distributions = "31c24e10-a181-5473-b8eb-7969acd0382f"
1110
DomainSets = "5b8099bc-c8ec-5219-889f-1d9e522a28bf"
12-
IfElse = "615f187c-cbe4-4ef1-ba3b-2fcf58d6d173"
1311
LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e"
1412
LogExpFunctions = "2ab3a3ac-af41-5b50-aa03-7779005ae688"
1513
Manifolds = "1cead3c2-87b3-11e9-0ccd-23c62b72b94e"
@@ -23,13 +21,11 @@ Symbolics = "0c5d862f-8b57-4792-8d23-62f2024744c7"
2321
[compat]
2422
DSP = "0.7, 0.8"
2523
DataStructures = "0.18, 0.19"
26-
Dictionaries = "0.4"
2724
Distributions = "0.25"
2825
DomainSets = "0.7, 0.8"
29-
IfElse = "0.1"
3026
LinearAlgebra = "1.10"
3127
LogExpFunctions = "0.3, 1.0"
32-
Manifolds = "0.9, 0.10, 0.11"
28+
Manifolds = "0.10, 0.11"
3329
PDMats = "0.11"
3430
PrecompileTools = "1"
3531
RecursiveArrayTools = "3, 4"

src/atoms.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,7 @@ Returns the log of the normal cumulative distribution function of `x`.
420420
- `x::Real`: A Real.
421421
"""
422422
function lognormcdf(x::Real)
423-
return logcdf(Normal, x)
423+
return logcdf(Normal(), x)
424424
end
425425
Symbolics.@register_symbolic lognormcdf(x::Real)
426426
add_dcprule(lognormcdf, RealLine(), Negative, Concave, Increasing)

src/gdcp/gdcp_rules.jl

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ function find_gcurvature(ex)
101101
if iscall(ex)
102102
f, args = operation(ex), arguments(ex)
103103
knowngcurv = false
104+
f_curvature = GUnknownCurvature
105+
f_monotonicity = (GAnyMono,)
104106

105107
if hasgdcprule(f) && !any(iscall.(args))
106108
rule, args = gdcprule(f, args...)
@@ -170,7 +172,7 @@ function find_gcurvature(ex)
170172
elseif argscurv == GConcave
171173
return GConvex
172174
else
173-
argscurv
175+
return argscurv
174176
end
175177
else
176178
@warn "Disciplined Programming does not support multiple non-constant arguments in multiplication"

src/gdcp/lorentz.jl

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,19 +16,20 @@ using Symbolics: Symbolic, @register_symbolic, unwrap, variables
1616
add_gdcprule(Manifolds.distance, Manifolds.Lorentz, Positive, GConvex, GAnyMono)
1717

1818
"""
19-
lorentz_log_barrier(a, p)
19+
lorentz_log_barrier(p)
2020
21-
Computes the log-barrier function for the Lorentz model: `-log(-1 - <a, p>_L)`.
21+
Computes the log-barrier function for the Lorentz model: `-log(-1 - <a, p>_L)`,
22+
with the fixed vector `a = (0, ..., 0, 1)` in R^(d+1).
2223
2324
# Arguments
2425
25-
- `a`: The vector (0, ..., 0, 1) in R^(d+1).
2626
- `p`: A point on the Lorentz manifold.
2727
"""
2828
function lorentz_log_barrier(p::AbstractVector)
29-
# Lorentzian inner product: a⋅p_L = a1*p1 + ... + a_d*p_d - a_{d+1}*p_{d+1}
30-
inner_prod = a[end] * p[end]
31-
return -log(-1 + inner_prod)
29+
# a = (0, ..., 0, 1), so the Lorentzian inner product
30+
# <a, p>_L = a1*p1 + ... + a_d*p_d - a_{d+1}*p_{d+1} reduces to -p[end].
31+
# The barrier is -log(-1 - <a, p>_L) = -log(-1 + p[end]).
32+
return -log(-1 + p[end])
3233
end
3334

3435
@register_symbolic lorentz_log_barrier(p::Union{Symbolics.Arr, AbstractVector})

src/rules.jl

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ struct CustomDomain{T} <: Domain{T}
77
end
88

99
Base.in(x, c::CustomDomain) = c.in(x)
10+
# Disambiguate against Symbolics' `in(::Num/::Symbolic, ::Domain)`, since
11+
# `CustomDomain <: Domain` makes the symbolic-variable calls match both methods.
12+
Base.in(x::Union{Symbolics.Num, Symbolic{<:Number}}, c::CustomDomain) = c.in(x)
13+
Base.in(x::NTuple{N, Union{Symbolics.Num, Symbolic{<:Number}}}, c::CustomDomain) where {N} = c.in(x)
1014

1115
function array_domain(element_domain)
1216
return CustomDomain{AbstractArray}() do xs
@@ -332,7 +336,7 @@ function find_curvature(ex)
332336
elseif argscurv == Concave
333337
return Convex
334338
else
335-
argscurv
339+
return argscurv
336340
end
337341
else
338342
@warn "DCP does not support multiple non-constant arguments in multiplication"

test/qa/qa.jl

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,37 @@
1-
using SymbolicAnalysis, Aqua, JET, Test
1+
using SymbolicAnalysis, Aqua, JET, SciMLTesting
22

3-
@testset "Aqua" begin
4-
Aqua.test_all(SymbolicAnalysis)
5-
end
3+
const SA = SymbolicAnalysis
64

7-
@testset "JET" begin
8-
JET.test_package(SymbolicAnalysis; target_defined_modules = true)
9-
end
5+
# Functions this package deliberately extends with symbolic-analysis methods.
6+
# Registering DCP/GDCP methods on these non-owned functions (and the Symbolics
7+
# traversal/symtype hooks) is the core purpose of the package, so they are
8+
# intentional and declared to Aqua's piracy check via `treat_as_own`.
9+
const SYMBOLIC_OWN = Any[
10+
Base.log, SA.LinearAlgebra.tr, SA.LinearAlgebra.inv, SA.LinearAlgebra.sqrt,
11+
SA.LinearAlgebra.logdet, SA.Manifolds.distance, SA.LogExpFunctions.xlogx,
12+
SA.Symbolics.arguments, SA.Symbolics.hasmetadata, SA.Symbolics.promote_symtype,
13+
]
14+
15+
# The scalar/array `@register_symbolic`/`@register_array_symbolic` registrations
16+
# for the package's own atoms generate overlapping signatures (e.g. a scalar and
17+
# an array method of the same atom), which Aqua reports as internal ambiguities.
18+
# These are all between SymbolicAnalysis's own methods (no cross-package
19+
# ambiguity remains) and are benign; concrete calls dispatch unambiguously.
20+
# Excluding only these atoms keeps the check live for everything else.
21+
const ATOM_AMBIGUITIES = Any[
22+
SA.affine_map, SA.sdivergence, SA.lorentz_least_squares, SA.conjugation,
23+
SA.log_quad_form, SA.lorentz_homogeneous_quadratic, SA.hadamard_product,
24+
SA.lorentz_homogeneous_diagonal, SA.lorentz_transform, SA.quad_over_lin,
25+
]
26+
27+
run_qa(
28+
SymbolicAnalysis;
29+
Aqua = Aqua,
30+
JET = JET,
31+
jet = true,
32+
aqua_kwargs = (;
33+
ambiguities = (; exclude = ATOM_AMBIGUITIES),
34+
piracies = (; treat_as_own = SYMBOLIC_OWN),
35+
),
36+
jet_kwargs = (; target_modules = (SymbolicAnalysis,), mode = :typo),
37+
)

0 commit comments

Comments
 (0)