Skip to content
This repository was archived by the owner on Mar 25, 2025. It is now read-only.

Add CvodeBlock and CvodeVisitor#1467

Merged
JCGoran merged 55 commits into
masterfrom
jelic/cvode_visitors
Oct 28, 2024
Merged

Add CvodeBlock and CvodeVisitor#1467
JCGoran merged 55 commits into
masterfrom
jelic/cvode_visitors

Conversation

@JCGoran

@JCGoran JCGoran commented Sep 24, 2024

Copy link
Copy Markdown
Contributor

Part 1 of #1399.
Adds the CVODE block since NMODL solves the DERIVATIVE block in-place, and we lose information about it. Also adds a visitors which actually replaces statements of the form x' = f(x) into Dx = f(x).

@JCGoran JCGoran changed the title Add DerivativeOriginalFunctionBlock and DerivativeVisitor Add DerivativeOriginalFunctionBlock and DerivativeOriginalVisitor Sep 24, 2024
bbpadministrator pushed a commit to BlueBrain/nmodl-references that referenced this pull request Sep 24, 2024
@JCGoran JCGoran marked this pull request as ready for review September 24, 2024 18:04

@1uc 1uc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two comments:

  • The DerivativeOriginalBlock is advertised as an unmodified copy. However, it's immediately during the process it's being modified.
  • There's no code demonstrating how this works, i.e. no tests or anything similar.

Comment thread src/visitors/derivative_original_visitor.hpp Outdated
Comment thread src/visitors/derivative_original_visitor.hpp Outdated
Comment thread src/visitors/derivative_original_visitor.cpp Outdated
Comment thread src/visitors/derivative_original_visitor.cpp Outdated
Comment thread src/language/codegen.yaml Outdated
Comment thread src/language/codegen.yaml Outdated
bbpadministrator pushed a commit to BlueBrain/nmodl-references that referenced this pull request Sep 25, 2024
Goran Jelic-Cizmek added 2 commits September 25, 2024 11:32
`DERIVATIVE` blocks can't have array variables in NOCMODL by default, so
let's go with that.
bbpadministrator pushed a commit to BlueBrain/nmodl-references that referenced this pull request Sep 25, 2024
@JCGoran JCGoran changed the title Add DerivativeOriginalFunctionBlock and DerivativeOriginalVisitor Add CvodeBlock and CvodeVisitor Sep 27, 2024
bbpadministrator pushed a commit to BlueBrain/nmodl-references that referenced this pull request Sep 27, 2024
Comment thread src/visitors/derivative_original_visitor.hpp Outdated
Comment thread src/visitors/derivative_original_visitor.hpp Outdated
Comment thread src/visitors/derivative_original_visitor.hpp Outdated
Comment thread test/unit/visitor/derivative_original.cpp Outdated
Comment thread test/unit/visitor/derivative_original.cpp Outdated
Comment thread src/visitors/derivative_original_visitor.cpp Outdated
Comment thread src/visitors/derivative_original_visitor.cpp Outdated
@JCGoran JCGoran marked this pull request as draft September 30, 2024 08:08
bbpadministrator pushed a commit to BlueBrain/nmodl-references that referenced this pull request Oct 14, 2024
bbpadministrator pushed a commit to BlueBrain/nmodl-references that referenced this pull request Oct 14, 2024
bbpadministrator pushed a commit to BlueBrain/nmodl-references that referenced this pull request Oct 15, 2024
bbpadministrator pushed a commit to BlueBrain/nmodl-references that referenced this pull request Oct 15, 2024
bbpadministrator pushed a commit to BlueBrain/nmodl-references that referenced this pull request Oct 16, 2024

@1uc 1uc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks quite good, a couple of last minor comments.

Comment thread src/visitors/cvode_visitor.cpp Outdated
Comment thread src/visitors/cvode_visitor.cpp Outdated
Comment thread src/visitors/cvode_visitor.cpp Outdated
Comment thread src/visitors/cvode_visitor.cpp Outdated
Comment thread src/visitors/cvode_visitor.cpp Outdated
Comment thread src/visitors/cvode_visitor.cpp Outdated
Comment thread src/visitors/cvode_visitor.cpp Outdated
Comment thread src/visitors/cvode_visitor.cpp Outdated
@JCGoran JCGoran merged commit 6b6a630 into master Oct 28, 2024
@JCGoran JCGoran deleted the jelic/cvode_visitors branch October 28, 2024 07:57
JCGoran added a commit to neuronsimulator/nrn that referenced this pull request Mar 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants