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

Incorrect graphical output for some sedml tests #60

Closed
shalinshah1993 opened this Issue Jul 23, 2018 · 12 comments

Comments

Projects
5 participants
@shalinshah1993
Owner

shalinshah1993 commented Jul 23, 2018

The tests that do not give correct answers:

  • osci-nested-pulse
  • repeated-scan-osci
  • repeated-steady-scan
  • repeated-stochastic-scan
  • param-scan-2D

The graphical outputs are exactly the same for all the repeated tasks i.e. the individual iterations don't apply changes from the listOfChanges sedml element.

@shalinshah1993 shalinshah1993 self-assigned this Jul 23, 2018

@shalinshah1993 shalinshah1993 added this to todo in Support for SED-ML L1V2 via automation Jul 23, 2018

@shalinshah1993

This comment has been minimized.

Show comment
Hide comment
@shalinshah1993

shalinshah1993 Jul 23, 2018

Owner

The source of this bug is jlibsedml's ModelResolver class. As per description, SEDMLDocument.getModelString should apply all the list of changes to the model ( including repeatedTasks compute changes) but no changes are applied.

This is because jlibsedml's computeChange function only supports changing:

- CHANGE_ATTRIBUTE_KIND 
- REMOVE_XML_KIND
- ADD_XML_KIND
- NO_NAMESPACE
- CHANGE_XML_KIND

However, as per their documentation of Change class, there can be following categories of changes.

- SEDMLTags.CHANGE_ATTRIBUTE_KIND
- SEDMLTags.CHANGE_XML_KIND
- SEDMLTags.ADD_XML_KIND
- SEDMLTags.REMOVE_XML_KIND
- SEDMLTags.COMPUTE_CHANGE_KIND (missing in their implementation, required to comute model changes for each iteration of repeated tasks)
- SET_VALUE_KIND

@matthiaskoenig @draeger do you guys know who is working on jlibsedml who can respond to this issue?

I see Richard Adams maintains jlibsedml and he also wrote code for SBSCL in the past. Can you connect me with him?

Owner

shalinshah1993 commented Jul 23, 2018

The source of this bug is jlibsedml's ModelResolver class. As per description, SEDMLDocument.getModelString should apply all the list of changes to the model ( including repeatedTasks compute changes) but no changes are applied.

This is because jlibsedml's computeChange function only supports changing:

- CHANGE_ATTRIBUTE_KIND 
- REMOVE_XML_KIND
- ADD_XML_KIND
- NO_NAMESPACE
- CHANGE_XML_KIND

However, as per their documentation of Change class, there can be following categories of changes.

- SEDMLTags.CHANGE_ATTRIBUTE_KIND
- SEDMLTags.CHANGE_XML_KIND
- SEDMLTags.ADD_XML_KIND
- SEDMLTags.REMOVE_XML_KIND
- SEDMLTags.COMPUTE_CHANGE_KIND (missing in their implementation, required to comute model changes for each iteration of repeated tasks)
- SET_VALUE_KIND

@matthiaskoenig @draeger do you guys know who is working on jlibsedml who can respond to this issue?

I see Richard Adams maintains jlibsedml and he also wrote code for SBSCL in the past. Can you connect me with him?

@draeger

This comment has been minimized.

Show comment
Hide comment
@draeger

draeger Jul 24, 2018

Collaborator

I don't currently have any contact to @otter606 and am not aware of the current state of jlibsedml.

Collaborator

draeger commented Jul 24, 2018

I don't currently have any contact to @otter606 and am not aware of the current state of jlibsedml.

@otter606

This comment has been minimized.

Show comment
Hide comment
@otter606

otter606 Jul 24, 2018

otter606 commented Jul 24, 2018

@draeger

This comment has been minimized.

Show comment
Hide comment
@draeger

draeger Jul 25, 2018

Collaborator

Maybe @fbergmann can give more information?

Collaborator

draeger commented Jul 25, 2018

Maybe @fbergmann can give more information?

@fbergmann

This comment has been minimized.

Show comment
Hide comment
@fbergmann

fbergmann Jul 25, 2018

I am not actively using jlibsedml. I'd be happy to propagate any code changes to the repository, or add another developer to the list. Chris Meyers was the last to commit to the project.

fbergmann commented Jul 25, 2018

I am not actively using jlibsedml. I'd be happy to propagate any code changes to the repository, or add another developer to the list. Chris Meyers was the last to commit to the project.

@draeger

This comment has been minimized.

Show comment
Hide comment
@draeger

draeger Jul 25, 2018

Collaborator

@cjmyers: what is the current state of jlibsedml?

Collaborator

draeger commented Jul 25, 2018

@cjmyers: what is the current state of jlibsedml?

@cjmyers

This comment has been minimized.

Show comment
Hide comment
@cjmyers

cjmyers Jul 25, 2018

cjmyers commented Jul 25, 2018

shalinshah1993 added a commit that referenced this issue Jul 25, 2018

@shalinshah1993

This comment has been minimized.

Show comment
Hide comment
@shalinshah1993

shalinshah1993 Jul 26, 2018

Owner

Since no one actively managing jlibsedml, I just added my own code for working with SEDMLTags.COMPUTE_CHANGE_KIND. Currently, the code uses string processing functions to extract SBML parameter from XPath and it updates them if SetValue element specifies a range for the parameter.

After pushing b64b096

The test cases that don't work:

  • osci-nested-pulse (because functional range MathML evaluation not supported by jlibsedml yet)
  • repeated-stochastic-scan (same as first)
  • param-scan-2D (because SBSCL doesn't support repeated task)

Two test cases that are giving unstable results:

  • repeated-steady-scan-oscli
  • repeated-stochastic-runs

Their outputs are as under. My guess why we get different results is because of the use of different simulation algorithm. We use Rosenbrock solver instead of what the SED-ML files ask.

image
image

Owner

shalinshah1993 commented Jul 26, 2018

Since no one actively managing jlibsedml, I just added my own code for working with SEDMLTags.COMPUTE_CHANGE_KIND. Currently, the code uses string processing functions to extract SBML parameter from XPath and it updates them if SetValue element specifies a range for the parameter.

After pushing b64b096

The test cases that don't work:

  • osci-nested-pulse (because functional range MathML evaluation not supported by jlibsedml yet)
  • repeated-stochastic-scan (same as first)
  • param-scan-2D (because SBSCL doesn't support repeated task)

Two test cases that are giving unstable results:

  • repeated-steady-scan-oscli
  • repeated-stochastic-runs

Their outputs are as under. My guess why we get different results is because of the use of different simulation algorithm. We use Rosenbrock solver instead of what the SED-ML files ask.

image
image

@shalinshah1993

This comment has been minimized.

Show comment
Hide comment
@shalinshah1993

shalinshah1993 Jul 26, 2018

Owner
Owner

shalinshah1993 commented Jul 26, 2018

@fbergmann

This comment has been minimized.

Show comment
Hide comment
@fbergmann

fbergmann Jul 26, 2018

so far sources are kept in sf svn ... but if you send me a patch i will apply it happily

fbergmann commented Jul 26, 2018

so far sources are kept in sf svn ... but if you send me a patch i will apply it happily

@shalinshah1993

This comment has been minimized.

Show comment
Hide comment
@shalinshah1993

shalinshah1993 Jul 30, 2018

Owner

This depends on #55

Owner

shalinshah1993 commented Jul 30, 2018

This depends on #55

@shalinshah1993

This comment has been minimized.

Show comment
Hide comment
@shalinshah1993

shalinshah1993 Aug 6, 2018

Owner

This issue was moved to draeger-lab#7

Owner

shalinshah1993 commented Aug 6, 2018

This issue was moved to draeger-lab#7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment