Skip to content

Conversation

@ChrisRackauckas-Claude
Copy link
Contributor

Summary

  • Fix TypeError when calling jacobian with non-Vector inputs like SVector
  • Change Vector to AbstractVector in type checks and assertions in the jacobian function

Problem

The jacobian function at src/diff.jl:649-656 used Vector in type assertions:

if ops isa Vector{Num}
    ops = unwrap.(ops)::Vector{SymbolicT}

However, vec(scalarize(ops)) can return other AbstractVector subtypes (e.g., SVector), causing a TypeError: in typeassert, expected Vector{Num}, got a value of type SVector{2, Num}.

Solution

Replace Vector with AbstractVector in all type checks and assertions within the function to properly support all AbstractVector subtypes.

Test

using Symbolics
using StaticArrays

@variables x y
ops = SVector(x^2 + y, x*y)
vars = SVector(x, y)

J = Symbolics.jacobian(ops, vars)
# Now works correctly, returning:
# 2×2 Matrix{Num}:
#  2x  1
#   y  x

Fixes #1691

🤖 Generated with Claude Code

The jacobian function signature accepts any `ops` and `vars`, but the
type assertions inside required concrete `Vector` types. When using
`SVector` or other `AbstractVector` subtypes, `vec(scalarize(ops))`
returns the same type (not a `Vector`), causing a TypeError.

This changes `Vector` to `AbstractVector` in the type checks and
assertions to properly support all AbstractVector subtypes.

Fixes JuliaSymbolics#1691

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Nov 27, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 0% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 21.94%. Comparing base (9177273) to head (ff1b23e).
⚠️ Report is 21 commits behind head on master.

Files with missing lines Patch % Lines
src/diff.jl 0.00% 7 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

❗ There is a different number of reports uploaded between BASE (9177273) and HEAD (ff1b23e). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (9177273) HEAD (ff1b23e)
5 4
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1697       +/-   ##
===========================================
- Coverage   79.80%   21.94%   -57.86%     
===========================================
  Files          55       55               
  Lines        5159     5117       -42     
===========================================
- Hits         4117     1123     -2994     
- Misses       1042     3994     +2952     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Add test case for issue JuliaSymbolics#1691 to ensure jacobian works correctly with
SVector and other AbstractVector subtypes.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@ChrisRackauckas
Copy link
Member

@AayushSabharwal why are there so many type assertions in the first place?

@ChrisRackauckas ChrisRackauckas requested review from AayushSabharwal and removed request for AayushSabharwal December 5, 2025 07:38
@AayushSabharwal
Copy link
Member

Type stability, it helps with the benchmarks quite a bit.

@ChrisRackauckas
Copy link
Member

Because unwrap can return numbers?

@AayushSabharwal
Copy link
Member

Because scalarize doesn't infer, so vec(scalarize(..)) infers to Any, which means everything after that doesn't infer at all.

@ChrisRackauckas
Copy link
Member

Why wouldn't scalarize infer?

@AayushSabharwal
Copy link
Member

Given just BasicSymbolic there's nothing in the type which can tell the compiler whether scalarize returns a scalar or an array.

@ChrisRackauckas
Copy link
Member

Seems like it could be useful if scalarizing gave length 1 vectors?

@AayushSabharwal
Copy link
Member

Not really. collect uses scalarize under the hood and we want symbolic matrices to collect to matrices. I also can't imagine the amount of code that breaks from doing this 😅

@AayushSabharwal
Copy link
Member

AayushSabharwal commented Dec 5, 2025

I can (probably) turn scalarize into a different function called stable_scalarize which does what you suggest, and have scalarize just reshape the result. It might also help MTK. The function itself will have lots of dynamic dispatch because we need to call Term.f, but that's nothing new and unavoidable.

@ChrisRackauckas
Copy link
Member

Yeah because if this needs to asset Vector for type-stability, then it can't be abstracted. So it might need that utility at least for the differentiation functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeError in jacobian with non-Vector input

4 participants