-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for stochastic simulation added #54
Conversation
@hemilpanchiwala: Please clarify:
I see 5 errors (not fails) with NPE in the build log:
Also, tons of warnings of this type:
Or this:
Is this expected? |
@zakharc, the reason for Travis CI failing currently is that for the simulation of the SED-ML models, it downloads the models via a URN from biomodels. But currently, the site from which it downloads is down, so the tests are failing. This is what gets downloaded currently as a SEDML model <title>Apache Tomcat/8.0.24 - Error report</title><style type="text/css">H1 {font-family:Tahoma,Arial,sans-serif;color:white;background-color:#525D76;font-size:22px;} H2 {font-family:Tahoma,Arial,sans-serif;color:white;background-color:#525D76;font-size:16px;} H3 {font-family:Tahoma,Arial,sans-serif;color:white;background-color:#525D76;font-size:14px;} BODY {font-family:Tahoma,Arial,sans-serif;color:black;background-color:white;} B {font-family:Tahoma,Arial,sans-serif;color:white;background-color:#525D76;} P {font-family:Tahoma,Arial,sans-serif;background:white;color:black;font-size:12px;}A {color : black;}A.name {color : black;}.line {height: 1px; background-color: #525D76; border: none;}</style> So, once the site runs fine, these tests will pass fine. Also, the SBMLReader warnings are due to this as it is interpreting the above code as a model. |
For the tons of warnings of [VIOLATION]...., I don't have any idea about them currently as I have not worked on them. I will see that test file and try to figure out why this warnings come. |
As I said in my previous post (and highlighted in bold text):
I am quite sure, that throwing NPE on this scenario is not the best idea. If this is the reason for the errors, we need a meaningful exception for that.
I think re-triggering the build would be a good idea. I would like to see it pass :)
👍 |
Yes, I am talking about these tests only. I get the model as a string which I sent in my last message. Then this model is passed to SBMLReader which reads and only gives warnings (but simulation runs further). And at a point, when any field is accessed from this, it gets a null value. This is the reason why NPE is given. I have run the tests again but currently, also the tests are failing. But I am sure that the Travis will pass once this thing is running as the fern branch has a passing Travis in my forked repository. |
} | ||
|
||
} | ||
package fern.network.sbml; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am confused on this file. It says everything changed, but this is clearly code which existed before somewhere ?! Same with the example/Dsmts.java
above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matthiaskoenig, this happened due to different line separators in Windows (CRLF) and Ubuntu (LF). The file content remains the same only the line separators are different (as FERN is developed in Windows and I am working on Ubuntu).
…pdated download_bigg_models script
src/test/java/org/simulator/stochastic/StochasticTestSuiteTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/simulator/stochastic/StochasticTestSuiteTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/simulator/stochastic/StochasticTestSuiteTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/simulator/stochastic/StochasticTestSuiteTest.java
Outdated
Show resolved
Hide resolved
src/test/java/org/simulator/stochastic/StochasticTestSuiteTest.java
Outdated
Show resolved
Hide resolved
@zakharc, I have updated most of the requested changes. I have some questions in few which I have commented, once you answer that I will update that changes too. |
src/main/java/org/simulator/sedml/SedMLSBMLSimulatorExecutor.java
Outdated
Show resolved
Hide resolved
src/test/java/org/simulator/distance/MaxDivergenceToleranceTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added small changes but otherwise LGTM!
…tochasticTestSuiteTest
Updates from this PR:
StochasticTestSuiteTest
is added to SBSCL which simulates all the stochastic tests from the SBML Test Suite.With these updates and PR #53, stochastic simulation support is now added to SBSCL.
All the updates of the project are present at my blog posts at this link.