GSoC - SBML L3V2 models support with some other bug fixes

[hemilpanchiwala]

Major changes made in SBSCL (from GSoC start):

• Before this PR, two PRs have been merged to SBSCL. A complete description of changes made is described in that PRs.
  • PR #40
  • PR #37
• Absolute as well as Relative tolerances are now fixed for the simulation of SBML models. This has solved the problems of failing test cases of the SBML Test Suite due to small deviations.
• Now, SBML models with fbc extensions are simulated using FluxBalanceAnalysis in SBMLTestSuiteTest and SBMLTestRunnerWrapper.
• Active objective function variable is added to FluxBalanceAnalysis. And now all the fbc models in the SBML Test Suite pass successfully.
• SBML L3V2 models issues solved:
  • SBML L3V2 models do not have a fast property which will be handled now.
  • SBML L3V2 models now can have constraints, kinetic laws, events, species, rules (assignment rules and rate rules), initial assignments, and any others without math. This has been handled now through this PR in SBSCL.
  • Logical operators are now added to compileDouble in ASTNodeValue.
  • New functions are added to the ASTNodeInterpreter class. (issue #42)
  • Reaction without reactants and products has been solved now. (issue #41)
  • RELATIONAL_GT, RELATIONAL_GEQ, RELATIONAL_EQ, RELATIONAL_NEQ, RELATIONAL_LEQ, and RELATIONAL_LT are now updated to have multiple children.
  • SBSCL now supports rateOf csymbol that is newly added in SBML L3V2.
  • New variables have been added to RateRuleValue and RuleValue classes.
  • A new array changeRate has been added to SBMLinterpreter class that keeps the value of the derivatives at the current time.
  • constantHash has also been added to SBMLinterpreter that has key as variables and values are boolean (true if constant else false).
  • After making this changes, about 67 SBML L3V2 tests fail excluding the comp tests from the SBML Test Suite.
• Many other small issues have been resolved in this PR.

This PR closes #38, closes #41, closes #42, closes #43. More information about changes made till now in SBSCL can be found at my blog posts [Link].

[hemilpanchiwala]

You have a ton of formating changes of comments in this pull request which makes it very hard to review the actual changes. In the future do separate pull requests for formating changes (of the 98 changed files only ~5-6 were actually changed)

@matthiaskoenig, will take care of it from next time.

[hemilpanchiwala]

@hemilpanchiwala: Some of the comments are not related to your code. Please address the comments related to your changes first. For the rest: I am OK if you will adjust the rest during the next phase.

Small comments for the future:

• Please always exclude the formatting changes to the separate PR. I've seen a lot of whitespace changes/etc. in this PR. This will only add work for the reviewer and is not interesting at all.
• I would suggest looking into coding conventions and guidelines of Eclipse Foundation (if you didn't do this already): https://wiki.eclipse.org/Development_Conventions_and_Guidelines

Nice work.

@zakharc, Extremely sorry for adding the formatting in this PR. I will take care from next time. And, I have formatted the code according to the template provided by the JSBML community linked here.

[hemilpanchiwala]

@zakharc, I have updated most of the requested changes. I will soon update the SBMLTestSuiteRunnerWrapper class by dividing the code into different methods. And for the JavaDoc comments of the overridden methods and some others, I will update it as I work. However, all comment changes pointed by you have been updated.

[shalinshah1993]

Lots of good work here!

• For the white space diffs, it seems that they can be hidden away by clicking on settings and avoid white space changes. However, to avoid pushing them in commits in future, you need to change git attributes. Please add these lines in the .gitattributes file. It is the default template for Java code to alway convert the code to and will avoid the white space diffs. You can find default template for other languages here - https://github.com/github/gitignore

# avoid
text=auto 
eol=lf

# Java sources
*.java          text diff=java
*.gradle        text diff=java
*.gradle.kts    text diff=java

# These files are text and should be normalized (Convert crlf => lf)
*.css           text diff=css
*.df            text
*.htm           text diff=html
*.html          text diff=html
*.js            text
*.jsp           text
*.jspf          text
*.jspx          text
*.properties    text
*.tld           text
*.tag           text
*.tagx          text
*.xml           text

# These files are binary and should be left untouched
# (binary is a macro for -text -diff)
*.class         binary
*.dll           binary
*.ear           binary
*.jar           binary
*.so            binary
*.war           binary
*.jks           binary

• I saw some coding convention pointed out up here, while I agree it is a good practice to adhere to them, I was thinking may be we can incorporate style checks. For python, I know pylint can be easily incorporated with Jenkins. A quick search for java showed checkstyle. https://checkstyle.sourceforge.io/ @draeger how the timeline for this project, may be if it is really easy, we can incorporate lint checker with maven and travis?

• I also see that that one test is failing. Before this gets merged, can you either fix the test or for now comment it out so it doesn't break the master branch?

[hemilpanchiwala]

@shalinshah1993, I will add the snippet you gave in the .gitattributes file in the next commit. But for the tests, I don't think there is a single failing test instead there are many SBML Test Suite tests that are failing currently due to some SBML L3V2 and other issues.

[hemilpanchiwala]

Why did subclassing not work? I.e. only overwriting the solve method?

@matthiaskoenig, It did not work as the new instance of GlpkSolver was made in the solve() method explicitly. We did not have any reference to that class in SBSCL.

Here, the thing is going on as follows: We make an object of LinearProgramSolver (which is an interface) that is implemented by GLPKSolver class where the solve() method implementation is found. In this solve() method, scpsolver creates an instance of the GlpkSolver class which was creating all the problems and so making that instance (of GlpkSolver) null using deleteProb() solves the issue. But this is all in scpsolver so I made a copy of the GLPKSolver class with making the instance of GlpkSolver null on completing its usage. Once scpsolver makes this change then we can remove this class.

Note: GLPKSolver and GlpkSolver are different classes in scpsolver.

[matthiaskoenig]

Will merge this as soon as the minor changes & the test case are there.

[hemilpanchiwala]

Please add a FluxBalanceAnalysis test which:

• takes the ecoli core model, simulates a FBA, compares objective; currently a simple test case for FBA outside of the SBMLTestSuite is missing

@matthiaskoenig, I have updated the changes. And FBA test was there but it was named CobraSolverTest and was ignored by @Ignore tag. I have renamed it to FluxBalanceAnalysisTest and also removed the ignore tag.