RuleWorld / bionetgen

Rule-based modeling framework
https://bionetgen.org/
MIT License
59 stars 25 forks source link

Permit the use of ln() in the .bngl file. #182

Closed rhclark closed 7 years ago

rhclark commented 7 years ago

Line 1562 of Expression.pm contains a list of built-in functions that BioNetGen can handle. The line looks like this: if ( $fcn_name =~ /^(sin|cos|exp|log|abs|sqrt|floor|ceil)$/ ) I'm suggesting that we add ln to the list. There is code elsewhere in the Expression.pm file that converts ln() to log(), so it's clear that there was an intention to handle ln() at one time. My suggestion is that we simply add ln to the list. But since I don't know much about the downstream effects of this change, I thought I should let you guys scrutinize it a bit. What does everyone think ? (Oh yes.... The motivation for this is a partial .bngl file that I got from kai_loell that uses ln() in the energy pattern section. What do you guys think ?

Response from Jim Faeder:

This would be fine. I think the parser would have to be updated as well. The Network and NFsim code may also need to be updated if ln is for some reason not enabled by default in mu parser.

lh64 commented 7 years ago

'ln' is already supported by BNG (see http://bionetgen.org/index.php/BNGManual:Table_2). Can you forward the BNGL file that's throwing the error? It's possible that an older part of the code is being incorrectly used somewhere.

rhclark commented 7 years ago

Sure. I'll attach the file.
eEGFR_simplified.bngl.txt

lh64 commented 7 years ago

Fixed here: https://github.com/RuleWorld/bionetgen/commit/2160c8e4d2a52ac4768874cae33dc06fecb749b2