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

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

Open
jonrkarr opened this issue Jun 5, 2021 · 3 comments

Comments

@jonrkarr
Copy link

jonrkarr commented Jun 5, 2021

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:

107: ('variable_step_size', bool), # whether or not the algorithm proceeds with an adaptive step size or not

@jonrkarr
Copy link
Author

jonrkarr commented Aug 23, 2021

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.

@jonrkarr jonrkarr changed the title Recognize KISAO:0000656 in addition to KISA0:0000107 Delete mapping from KiSAO to variable step size for SED-ML UniformTimeCourse Aug 23, 2021
@luciansmith
Copy link
Contributor

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
Copy link
Author

jonrkarr commented Nov 1, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants