-
Notifications
You must be signed in to change notification settings - Fork 16
Handle after_cvode solver and at_time declaration #406
Conversation
- Added function to return if NrnThread arg should be added or not - Added new definition for at_time
| auto name = node.get_block_name()->get_node_name(); | ||
| const auto& method = node.get_method(); | ||
| auto method = node.get_method(); | ||
| if (method != nullptr && method->get_node_name() == codegen::naming::CVODE_METHOD) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we plan to have cvode in the future in coreneuron or not?
cattabiani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good PR! A few nitpicking comments...
pramodk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just few minor nitpicks otherwise lgtm!
| * in multiple context and hence we are keeping original names. | ||
| * Once we finish code generation part then we change this. | ||
| */ | ||
| static std::map<std::string, TokenType> keywords = {{"VERBATIM", Token::VERBATIM}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you used different/newer clang-format? If this is required, we should do that in different PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've used clang-format-9 for the formatting, however it might not be the version that changed the formatting but the addition of const in the beginning. Then {{"VERBATIM", Token::VERBATIM}, doesn't fit in the end of the line and the other entries cannot be placed underneath it
| {"cvode_t", MethodInfo(0, 0)}, | ||
| {"cvode_t_v", MethodInfo(0, 0)}}; | ||
| const static std::map<std::string, MethodInfo> methods = {{"adams", MethodInfo(DERF | KINF, 0)}, | ||
| {"runge", MethodInfo(DERF | KINF, 0)}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar reason as before. Addition of const in the beginning of the line should be the reason for the change of formatting
cattabiani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
g2g!
Co-authored-by: Pramod Kumbhar <[email protected]>
pramodk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
- Change `after_cvode` solver to `cnexp` - Add `nt` variable in `at_time` function NMODL Repo SHA: BlueBrain/nmodl@528043d
after_cvodemethod tocnexpntargument to functions needing them, likeat_time