Skip to content
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

simplify initial condition export and preserve parameter dependency #417

Merged
merged 5 commits into from Feb 15, 2019

Conversation

Projects
None yet
3 participants
@FFroehlich
Copy link
Contributor

commented Feb 8, 2019

If we directly write the values of Parameters/Expression as InitialAmounts or InitialAssignment we lose the dependency on the original entities. Also simplifies code a bit.

@FFroehlich

This comment has been minimized.

Copy link
Contributor Author

commented Feb 11, 2019

As there is no way of having a non-Parameter/Expression value for Initials it is probably not advisable to always encode them as symbolic entities. Yet, with the previous implementation it was impossible to create parameter dependent intial conditions from pysb. As a compromise, this PR implements the encoding of Initials with Expression values as symbolic references in sbml instead of expanding them and writing the flat values. This allows creation of parametric initial conditions by creating an Expression for every initial condition parameter.

@coveralls

This comment has been minimized.

Copy link

commented Feb 11, 2019

Coverage Status

Coverage decreased (-0.2%) to 78.908% when pulling 8e0ca20 on FFroehlich:sbml_export_initial_conditions into 8e67aea on pysb:master.

@alubbock
Copy link
Member

left a comment

Personally I'd lean towards exporting parameter initials to SBML as symbols too, since that best reflects the PySB model. But it's a matter of opinion, so I'm fine if you want to leave it as is.

Should probably remove the duplicate if is.fixed block before merging, though.

_check(ia.setMath(init_mathml))
initial_concs[sp_idx] = None
else:
initial_concs[sp_idx] = ic.value.value
if ic.fixed:

This comment has been minimized.

Copy link
@alubbock

alubbock Feb 11, 2019

Member

Should remove this if ic.fixed block, since it's now duplicated below

@FFroehlich

This comment has been minimized.

Copy link
Contributor Author

commented Feb 11, 2019

Okay changed such that both Expressions and Parameters will be written as initial condition

@alubbock

This comment has been minimized.

Copy link
Member

commented Feb 12, 2019

Caught a couple of .pytest_cache files in the commit. The Appveyor build was also failing due to a separate issue with the stochkit-lite and BNG anaconda packages, which I've now fixed. Hopefully if you re-push with the .pytest-cache files removed, the new build will go through.

FFroehlich added some commits Feb 12, 2019

@alubbock alubbock merged commit 8ebf62c into pysb:master Feb 15, 2019

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.2%) to 78.908%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.