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

fix(sbml-import): make TimeDerivative rhs have a per_time dimension #24

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

sanjayankur31
Copy link
Member

Otherwise we end up with dimensionally incorrect values:

<TimeDerivative variable="time" value="1"/>

It'll now be:

<TimeDerivative variable="time" value="tscale * 1"/>

where tscale has dimensions per_time.

Otherwise we end up with dimensionally incorrect values:

```
<TimeDerivative variable="time" value="1"/>
```

It'll now be:

```
<TimeDerivative variable="time" value="tscale * 1"/>
```

where `tscale` has dimensions `per_time`.
@pgleeson
Copy link
Member

Great! Does the resultant file look like an izh cell? Test also the SBMLshowcase locally with this too.

@pgleeson
Copy link
Member

FYI this fails for me currently, should be fixed with this PR: https://github.com/OpenSourceBrain/SBMLShowcase/blob/master/regenerate.sh

@sanjayankur31
Copy link
Member Author

I'll go test it out with the SBML showcase next. Here's the generated izhikevich lems file:

Izhikevich_LEMS.xml.txt

Runs OK from the looks of it:

image

@sanjayankur31
Copy link
Member Author

FYI this fails for me currently, should be fixed with this PR: https://github.com/OpenSourceBrain/SBMLShowcase/blob/master/regenerate.sh

yeh, seems to run fine with this PR:

sbml-showcase-regenerate.txt

@pgleeson pgleeson merged commit 44cfa92 into experimental Jun 14, 2024
18 checks passed
@pgleeson
Copy link
Member

Cheers!

@sanjayankur31 sanjayankur31 deleted the fix/sbml-derivative branch June 15, 2024 00:21
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

Successfully merging this pull request may close these issues.

2 participants