SymbolicNumericIntegration.jl

SymbolicNumericIntegration.jl copied to clipboard

Minor amount of type piracy needs to be fixed.

Dec 02 '23 18:12 ArnoStrouwen

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (7b3920c) 57.06% compared to head (296678b) 57.64%.

@@            Coverage Diff             @@
##             main      #77      +/-   ##
==========================================
+ Coverage   57.06%   57.64%   +0.58%     
==========================================
  Files          13       13              
  Lines        1055     1053       -2     
==========================================
+ Hits          602      607       +5     
+ Misses        453      446       -7     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Dec 02 '23 18:12 codecov[bot]

@AlCap23 does it make sense to upstream this function? https://github.com/SciML/SymbolicNumericIntegration.jl/blob/7b3920c87d5bf667697f163f7d9332fb5e3b9d48/src/sparse.jl#L124-L128

Dec 30 '23 15:12 ArnoStrouwen

@ChrisRackauckas If tests pass, I think this is good to go, the last remaining piracy can be made into an issue and fixed later. EDIT: nevermind, this package has weird tests, it does not seem to fail if some integrals can no longer be solved.

Dec 30 '23 15:12 ArnoStrouwen

@shahriariravanian is it safe to delete these functions? https://github.com/SciML/SymbolicNumericIntegration.jl/pull/77/files#diff-5286ab18446ceac577bfe10dbc94bafeb42424a99a1a1556e26aa7dd8ec26267 Since it is type piracy, it is very hard to track down what the intended use of these lines is.

Dec 30 '23 16:12 ArnoStrouwen

@ArnoStrouwen We still need Base.signbit(z::Complex{T}) where {T <: Number} = signbit(real(z)). This is to prevent error messages when it tries to calculate abs(z) for when z is a complex number.

The problem arises because some of the derivatives in Symbolics have an abs in them. For example, expand_derivatives(Differential(x)(asec(x))) returns 1 / (abs(x)*sqrt(-1 + x^2)), which is the standard solution for real arguments but is meaningless for complex arguments. I think a better option would be to fix Symbolics to return 1 / (x^2 * sqrt(1 - 1/x^2)), which is what Mathematica does and works for complex values. If Symbolics is fixed, we can get rid of this signbit line.

Dec 30 '23 20:12 shahriariravanian

That edit should happen in DiffRules.jl

Dec 31 '23 18:12 ChrisRackauckas