Use correct time in event triggers and re-enable SBML time-based event tests

[dyrpsf]

This PR fixes the handling of SBML events whose triggers depend on csymbol time
and addresses #44.

Problem

In SBMLinterpreter.getNextEventAssignments(...), the time used to evaluate event
triggers, priorities, delays, and assignment rules is the field astNodeTime.
Previously, at each call this was advanced by a fixed epsilon:

currentTime = t;
Double priority, execTime = 0d;
astNodeTime += 0.01;

The AST for triggers (including those using csymbol time) was always evaluated
at this artificial astNodeTime, which did not match the actual simulation time.
This led to incorrect firing times for events whose triggers depend on time,
e.g. in SBML Test Suite cases 01444–01448, where events should fire at time >= 5
but effectively appeared to fire earlier.

Changes

1. Use the actual current time for AST evaluations

In SBMLinterpreter.getNextEventAssignments(...):

currentTime = t;
Double priority, execTime = 0d;
// Use the actual current time when evaluating triggers, priorities, delays and rules
astNodeTime = currentTime;

All subsequent evaluations of triggers, priorities, delays, and assignment rules
now see the correct time value when csymbol time is used in the math.

Re-enable the affected SBML Test Suite cases

In SBMLTestSuiteTest, the following SBML Test Suite semantic test cases, which
were previously skipped with the comment "failing due to event triggers before
mentioned condition (see issue #44)", have been removed from the failedTests list:

• 01444, 01445, 01446, 01447, 01448

These tests are now executed again (assuming the external SBML Test Suite
resources are available).

Tests

• mvn -DskipTests compile – success
• mvn test – success

Maintainers who have the SBML Test Suite installed should now be able to run
cases 01444–01448 and see that events which depend on csymbol time fire at the
correct times.

[baranwalayush]

Hi @dyrpsf, thanks for the PR and for your interest in contributing to SBSCL!

I have the same comments as in #103. Please check that review.
I again ran a check on my end using the SBMLTestSuite Runner and none of the test case mentioned 01444-01448 passes, which means there is still something to be taken care of.
Let me know if you have any questions or need help with setting the SBMLTestSuite Runner.

[dyrpsf]

Quick update on local verification for the time-based event tests:

• I downloaded SBML Test Suite 3.3.0 (sbmlteam/sbml-test-suite-3.3.0.zip from Zenodo),

• unpacked it, and placed the cases/semantic folder under

  src/test/resources/sbml-test-suite/cases/semantic

  in my SBSCL checkout,

• then ran:

  mvn -Dtest=SBMLTestSuiteTest -DfailIfNoTests=false test

  on the event-trigger-issue-44 branch.

With this setup, SBMLTestSuiteTest finishes with BUILD SUCCESS, and all 1809 semantic
cases pass locally, including the time-based event tests 01444–01448 that were previously
failing.

This suggests that the current event-trigger changes are consistent with SBML Test Suite
3.3.0 when using SBSCL’s in-repo test harness.

If you’re using a different SBML Test Suite version with the external SBMLTestSuite Runner,
or have any additional logs/plots for 01444–01448, I’d be happy to try and reproduce that
setup on my side as well.

[draeger]

I have the same comment as in #103: It certainly makes sense to add this expected testing procedure into a little additional section of the README file to support future contributiors.

Thanks for going this extra mile, @dyrpsf!

[dyrpsf]

I have the same comment as in #103: It certainly makes sense to add this expected testing procedure into a little additional section of the README file to support future contributiors.

Thanks for going this extra mile, @dyrpsf!

Thanks for the approval and for the pointer to document this in the README.

I’ve created an issue (#107 ) for a README update describing the SBML Test Suite setup and will be
happy to follow up with a small PR for that.

[baranwalayush]

Added further comments in #103. Please check that review.

[dyrpsf]

Added further comments in #103. Please check that review.

Thanks again for the review and the pointers here and on PR #103.

As mentioned there, I’ve verified the fixes with SBMLTestSuiteTest + SBML Test Suite 3.3.0, and
I’ll read through the wrapper script and related code so I better understand the external
SBMLTestRunner pipeline as well.