allow 2-argument piecewise; use Base.ifelse

[paulflang]

No description provided.

[codecov]

Codecov Report

Patch coverage: 66.66% and project coverage change: -1.11 ⚠️

Comparison is base (f28935c) 94.49% compared to head (40dba78) 93.39%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #251      +/-   ##
==========================================
- Coverage   94.49%   93.39%   -1.11%     
==========================================
  Files          10       10              
  Lines         818      817       -1     
==========================================
- Hits          773      763      -10     
- Misses         45       54       +9     

Impacted Files	Coverage Δ
src/SBML.jl	100.00% <ø> (ø)
src/interpret.jl	74.19% <66.66%> (ø)

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[paulflang]

I just realised that the otherwise in piecewise is optional (https://www.w3.org/TR/MathML3/chapter4.html#contm.piecewise). I need sth like this PR to get this to work.

[exaexa]

🤔 single-branch if in formulas is IMO a blasphemy but if SBML allows that we should support it... actually what does the specification say for the cases when the formula is false and the else-branch is missing?

[paulflang]

Agreed that it is blasphemy, but it is MathML, not SBML to blame for it. Even worse, I cannot find in the text I linked above or elsewhere what should happen if the formula evaluates false and the else statement is missing. But I would assume that the intention is that the function is not defined in that case. For me this translates to NaN or a DomainError.

[exaexa]

Why would we remove IfElse? (This was dragged in to increase the compatibility wth rest of the MTK ecosystem, has stuff changed?)

[paulflang]

https://github.com/SciML/IfElse.jl, increased the compatibility with the rest of the MTK ecosystem, but is now archived since the functionality is in base (I believe).

[paulflang]

Or is this why the test is failing?

[exaexa]

Btw I'm afraid we're not going to find any good material for this; the best I found is in MathML spec here: http://zvon.org/xxl/MathML/Output/el_piecewise.html . To interpret the missing <otherwise> part as an error, we would need Symbolics support for failure in the expression (aka undefined aka bottom aka ⊥ aka empty model set), but I assume there isn't any.

maybe if @ChrisRackauckas has a moment -- is there any good way to translate the under-definedness here to Symbolics?

in short:

• model says: if some_condition then it's 5 and no information about else
• if we wrote first-class julia, this would be some_condition ? 5 : error("hit empty else-branch")
• in embedded language (symbolics) this would then go: IfElse.ifelse(some_condition, 5, ????) (notably if you use error here, it's going to crash right away)

[exaexa]

https://github.com/SciML/IfElse.jl, increased the compatibility with the rest of the MTK ecosystem, but is now archived since the functionality is in base (I believe).

oh nice I didn't know. I'll remove it. (any info on what's the julia version where it got interned?)

Or is this why the test is failing?

it says

  Expression: isequal(interpret_as_num(test), 456)
  TypeError: non-boolean (Num) used in boolean context

looks like it interprets this as an Num expression which says something like :(test == 456). Which is not a testable boolean ofc. I guess isequal has an overload for Num.

Maybe you wanted interpret_as_num(test) === 456.

[exaexa]

Anyway yeah, the general problem here is that 1] MathML is meant as TeX encoded in XML and 2] SBML spec somehow seems to assume that MathML is interpretable as actual mathematics for free (it isn't). Let's wait for input from Symbolics folks and see.

[exaexa]

Closing in favor of #253