AlgebraicJulia / StockFlow.jl

https://algebraicjulia.github.io/StockFlow.jl/
MIT License
63 stars 6 forks source link

Fix bug in handling compound dynamic variable expressions #129

Closed Saityi closed 3 weeks ago

Saityi commented 4 weeks ago

Issue

Unary dynamic variable expressions that contain subexpressions, like exp(a + b + c), are currently erroneously rejected. A user reported issues with a snippet with this structure:

# Define the stock and flow model
model = @stock_and_flow begin
    ...
    :dynamic_variables
    h  = (exp(
        di +
        dcf * F + 
        dct * T +
        dcv * V + 
        dcvf * F * V
    ))
end

Solution

The 'simple' cases in dynamic variable assembling should only occur when the arguments are simple (i.e., non-expressions). If they are expressions, they should be recursively pulled apart to match the form required by the StockAndFlowF data type.

Testing

Macro usages such as above should now work. I gave the original example a try and it seems to work now -- at the very least, it no longer throws an error.

Unit tests were added which replicate the previous issue with is_binop_or_unary (renamed is_simple_dyvar).

All previous tests appear to still pass (locally).

codecov[bot] commented 4 weeks ago

Codecov Report

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

Project coverage is 60.69%. Comparing base (c7a0faf) to head (c6cb155). Report is 6 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #129 +/- ## ========================================== - Coverage 61.51% 60.69% -0.83% ========================================== Files 10 10 Lines 2193 2193 ========================================== - Hits 1349 1331 -18 - Misses 844 862 +18 ```

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