#268 No validation rule for indirectly recursive functions

[Lucian Smith]

The specification forbids indirectly-recursive function definitions, but there is no validation rule that test this (and indeed, libsbml claims that the attached model is valid).

If https://sourceforge.net/p/sbml/sbml-specifications/260/ is accepted, we still need a validation rule for L2v5, and if it's accepted but with the requirement that an attribute be used, we need the validation rule to check that attribute.

[Lucian Smith]

Since this seems to be a clear omission, I have taken the liberty of fixing the issue in SVN, though the editors still need to vote here. If a different solution is proposed, the SVN can be reverted and changed again.

If accepted, this change will be incorporated into both L2v5 and L3v2, upon the release of those specifications.

[Michael Hucka]

I agree with the proposed changes description in Lucian's 2nd paragraph.

[Frank Bergmann]

I agree this validation rule should be added. But preferably no recursive function definitions are added.

[Nicolas Le Novère]

I agree with the proposed changes

[Lucian Smith]

This has now been added to the list of known errata for both specifications, but still needs one more vote (Brett, Sven, and/or Dagmar) before being officially accepted.

[Brett Olivier]

I agree the validation rule should be added.

This does not imply that I necessarily agree with the inclusion of recursive function definitions.

[Lucian Smith]

And, that's a wrap! Thanks; the validation rule will be added to the upcoming L2v5 and L3v1 specifications.

[Sarah Keating]

Sorry to reopen this but I found an anomaly in the validation rules.

The fix for l2v5 was to add 'or mutually recursive' to rule 20303 that said SBML functions are not permitted to be recursive.

But L3V2 has a whole new rule (20308) that says they may not be mutually recursive. I think this is unecssaery and we should go with the L2V5 solution of explicilty mentioning it in rule 20303.

[Lucian Smith]

I found a discussion on this in the libsbml-team archives:

Sarah: "My current stance is that it should be an addition to rule 20303 NOT a new rule at all. I don't remember ever arguing the opposite but if I did then when I came to implement it I changed my mind !

Basically in implementing the rule if it were a separate rule I would have to pick out and quash any reports of just a single level recursion that is covered by 20303. So it seemed to me that it is not a separate thing at all - merely a bug in our writing and implementation of the rule in the first place :-)"

Mike: "Looking at 20303 and 20308, I agree with you.

Here's my attempt at revising 20303:

Inside the \token{lambda} of a \FunctionDefinition, the identifier of that \FunctionDefinition cannot appear as the value of a \token{ci} element. SBML functions are not permitted to be recursive \changed{or mutually recursive}. (References: L2V2 Section~3.5.3 and~4.3.2; L2V3 Sections~3.4.3 and~4.3.2; L2V4 Sections~3.4.3 and~4.3.2\changed{; L2V5 Sections~\ref{sec:ci-token} and~\ref{sec:functiondefinition}}.)

Here's what 20308 previously said, before I removed it from the latest copy of the spec:

Inside the \token{lambda} MathML element within a \FunctionDefinition object, mutually recursive functions are not allowed. \FunctionDefinition \token{id} attributes may not appear as identifiers in \token{lambda} MathML elements if that appearance would cause a self-referential loop of function definitions. (References: SBML L2V5 Section~\ref{sec:functiondefinition}.)

Does the change look like it covers 20308?"

Sarah: "Fine by me :-)"

[Lucian Smith]

I think it's clear that the editors voted "This situation should be covered by the validation rules," so however we do that is fine. I'll make the change to the L3v2 spec.

[Lucian Smith]

Updated http://sbml.org/Documents/Specifications/SBML_Level_3/Version_1/Core/Confirmed_issues_in_the_Level_3_Version_1_Core_Specification#Release_1_.28Final.29 to say that we're modifying validation rule 20303 instead of adding a new validation rule.

[Lucian Smith]

(Also, checked in the new version into SVN--the change will be part of the forthcoming L3v2 specification.)

[Lucian Smith]

With the release of l3v2 (with a revised 20303 only, and no new 20308), this issue is now resolved.