SED-ML / sed-ml

Simulation Experiment Description Markup Language (SED-ML)
http://sed-ml.org
5 stars 2 forks source link

Change 'xError' and 'yError' to 'xErrorUpper' and 'yErrorUpper #137

Closed luciansmith closed 3 years ago

luciansmith commented 3 years ago

Matthias noticed that libsedml implemented xErrorUpper and yErrorUpper instead of the spec-defined 'xError' and 'yError'. In the spec, it claims that if you only set xError, you are to assume symmetric errors, and if you define both, it uses 'xError' as the upper one.

(However, the spec actually defines 'xErrorUpper' in the UML diagrams, just not in the text. So something clearly needs to be fixed.)

Upon reflection, I now feel like this is a bad idea. If you have symmetric errors, it's very easy to simply define both xErrorUpper and xErrorLower to point to the same dataGenerator. In addition, using upper/lower allows you to define only the upper error, which is impossible in the current spec text.

Any other opinions?

matthiaskoenig commented 3 years ago

Personally I think upper and lower errors are an overkill. The large majority of use cases has symmetrical errors (SD or SE). Also most plotting libraries support only a symmetrical error by default (and require custom code for plotting assymetrical ones). For instance both matplotlib and altair support only symmetrical errorbars by default.

I would simplify the specification and just use: xError and yError. I never needed asymetrical errors and only very rarely used xErrors. So to get better adoption of the specification I would suggest reducing complexity here and only go with xError and yError.

fbergmann commented 3 years ago

I think the whole plotting part of L1V4 got out of hand. But i find no good reason to argue against including asymmetric errors. but i'm fine with either verison.

agarny commented 3 years ago

It has gone out of hand sometime ago already. 🙂