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

Add fixed species support #364

Merged
merged 9 commits into from Feb 5, 2019

Conversation

Projects
None yet
3 participants
@jmuhlich
Copy link
Member

commented May 30, 2018

This adds a fixed boolean kwarg to Initial for declaring that a species should not be consumed or produced by any reactions. This maps to a leading $ on a species entry in BNG. If there's a Kappa equivalent I would be happy to add that too, but right now the KappaGenerator raises an exception.

The implementation adds a new list of booleans to Model, initial_conditions_fixed, rather than modify the current initial_conditions tuple structure. I'm not totally happy with this, though, and am open to alternatives. The effect of a fixed species is simply that its row in the stoichiometric matrix is set to zero.

@coveralls

This comment has been minimized.

Copy link

commented May 30, 2018

Coverage Status

Coverage decreased (-0.2%) to 79.078% when pulling 52f1e5c on jmuhlich:fixed_species into 470945e on pysb:master.

@alubbock

This comment has been minimized.

Copy link
Member

commented May 30, 2018

My suggestion, assuming you don't want to break API compatibility:

  • Turn Initial into a proper class, which stores the pattern, initial value parameter/expression, and fixed flag in its instances. That way, we can extend it with new attributes in the future without breaking API compatibility.
  • Store a list of Initials in Model.initials (or use @property to achieve same)
  • Turn Model.initial_conditions() into a compatibility shim/wrapper for Model.initials and perhaps mark it as pending deprecation.

CC @lh64 in case he has input from BNG point of view

@jmuhlich

This comment has been minimized.

Copy link
Member Author

commented May 31, 2018

Agreed. I had in fact thought of making an Initial class but somehow missed that we could store them in initials and shim initial_condtions -- I was focused on how to store them in initial_conditions without breaking anything. It's actually lucky that for some reason I used initial_conditions when initially writing this. initials will nicely parallel the other class/container pairs (Monomer/monomers, Observable/observables, etc).

@jmuhlich jmuhlich force-pushed the jmuhlich:fixed_species branch 2 times, most recently from 40ef9de to 066e76f Jan 3, 2019

jmuhlich added some commits Oct 25, 2017

Add InitialCondition class and Model.initials
This finally replaces the old tuples of (pattern, value) with proper objects. A
read-only shim for the original `Model.initial_conditions` is provided, but
under a DeprecationWarning.
Correct formatting of fixed species in BNG export
Compartment labels for ComplexPatterns go before the "$" indicating
that the species is fixed. Previously I had put the "$" first.
Add fixed species support to the BNG importer
Additionally:
* Correct use of deprecated Model.initial in Builder
* Move many BNG validation models from the "with_force" list to the "regular"
  list

@jmuhlich jmuhlich force-pushed the jmuhlich:fixed_species branch from 4d1b49d to cf2827f Jan 7, 2019

@@ -188,7 +187,7 @@ def ode_rhs(self, t, y, p):
ydot = self.ydot
weave.inline(r'''%s''', ['ydot', 't', 'y', 'p'])
return ydot
""", 8) % (pad('\n' + code_eqs, 16) + ' ' * 16))
""", 8) % ('\n' + pad(code_eqs, 16) + ' ' * 16))

This comment has been minimized.

Copy link
@jmuhlich

jmuhlich Jan 7, 2019

Author Member

This just corrects some extraneous trailing whitespace on the first line of the output. I only noticed it because I re-generated robertson_standalone.py and git diff highlighted the trailing whitespace.

ia = smodel.createInitialAssignment()
_check(ia)
_check(ia.setSymbol('__s{}'.format(sp_idx)))
init_mathml = self._sympy_to_sbmlast(sympify(param.name))
init_mathml = self._sympy_to_sbmlast(ic.value)

This comment has been minimized.

Copy link
@jmuhlich

jmuhlich Jan 7, 2019

Author Member

sympify wasn't even needed here, the param object is a sympy.Symbol already.

@jmuhlich

This comment has been minimized.

Copy link
Member Author

commented Jan 7, 2019

Thanks again @alubbock for the gentle push to do the right thing here. :) It's ready for another look now.

@alubbock
Copy link
Member

left a comment

Issue #400 got me thinking: should the new Initial class store the param/expression in something like ic.param/ic.param_or_expr/ic.obj, and ic.value instead be a property which passes through the parameter value/numerical evaluation of the expression, as appropriate?

@jmuhlich

This comment has been minimized.

Copy link
Member Author

commented Jan 30, 2019

I'd rather have something on Parameter and Expression directly rather than provide this feature only in the context of Initial, since it could be helpful in other contexts. Also we probably don't want to facilitate a "transparent .value" concept TOO much, since it only operates on the nominal parameter values which are of limited utility in any serious application.

@alubbock

This comment has been minimized.

Copy link
Member

commented Feb 4, 2019

OK, let's address #400 separately, then.

@alubbock alubbock merged commit b19d85d into pysb:master Feb 5, 2019

2 of 3 checks passed

coverage/coveralls Coverage decreased (-0.7%) to 78.584%
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jmuhlich jmuhlich deleted the jmuhlich:fixed_species branch Feb 5, 2019

@alubbock alubbock referenced this pull request Feb 5, 2019

Closed

fixed-concentration species #12

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.