sys-bio / tellurium

Python Environment for Modeling and Simulating Biological Systems
http://tellurium.analogmachine.org/
Apache License 2.0
109 stars 36 forks source link

Delete mapping from KiSAO to variable step size for SED-ML UniformTimeCourse #528

Open jonrkarr opened 3 years ago

jonrkarr commented 3 years ago

Tellurium recognizes KISA0:0000107 for the boolean argument whether to use adaptive time steps. However, this is a term for an algorithm characteristic rather than an algorithm parameter.

I added an algorithm parameter term KISAO:0000656 for this purpose.

Tellurium should either replace KISAO:0000107 with KISAO:0000656 or recognize KISAO:0000656 as a "synonym" for KISAO:0000107.

Here's the relevant line: https://github.com/sys-bio/tellurium/blob/3e7cfb57d448ca507e5193470f7526c98f070188/tellurium/sedml/tesedml.py#L245

jonrkarr commented 3 years ago

Actually, the above line should be deleted because neither should be recognized in conjunction with SED-ML UniformTimeCourse.

KISAO_0000656 could be recognized for variable time courses described with the new SED-ML Analysis class.

luciansmith commented 3 years ago

OK, since the dictionary in question is just 'terms Tellurium recognizes', I added the new term instead of removing the old one. Eventually when we support the generic Analysis class, we'll want it in the list.

For now, there's just an inherent contradiction in providing a 'variable step size' parameter for a uniform time course; the proper thing to do would be to provide an error. I'll leave this open until that's done.

jonrkarr commented 3 years ago

Another possible middle ground is to raise a DeprecationWarning to discourage its use (e.g., to communicate that this would not be portable across tools) while retaining support.