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.

Sign up for GitHub

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

Jump to bottom

Initial code generation for NEURON #1078

Merged
merged 54 commits into from
Nov 29, 2023
Merged

Initial code generation for NEURON #1078

merged 54 commits into from
Nov 29, 2023

Conversation

iomaganaris
Copy link
Contributor

@iomaganaris iomaganaris commented Sep 25, 2023 •
edited
Loading

  • Separated CodegenCppVisitor to CodegnCppVisitor, CodegenCoreneuronCppVisitor and CodegenNeuronCppVisitor
  • CodegnCppVisitor should now define functions and members that are shared by the Neuron and CoreNeuron code generation only
  • Initial TODOs in Neuron code generation code for the 2023 NEURON Hackathon
  • Added simulator cli option with --neuron and --coreneuron flags. (CoreNeuron is the default codegen)
  • NMODL architecture introduction slides: https://docs.google.com/presentation/d/1lvGpU5p8lMQGqd9vKCX3kMiIidxz3oX-RLAzTOcBTtk/edit?usp=sharing

TODO

  • Rearrange CodegenNeuronCppVisitor similarly to the other codegen visitors
  • Create proper TODO comments on CodegenNeuronCppVisitor source file
  • Integrate changes from master
  • Create simple unit test for NEURON code generation

@codecov
Copy link

codecov bot commented Sep 25, 2023 •
edited
Loading

Codecov Report

Attention: 113 lines in your changes are missing coverage. Please review.

Comparison is base (11ec824) 88.67% compared to head (f3f041d) 88.18%.

Files Patch % Lines
src/codegen/codegen_neuron_cpp_visitor.cpp 65.64% 101 Missing ⚠️
src/main.cpp 52.94% 8 Missing ⚠️
src/codegen/codegen_neuron_cpp_visitor.hpp 50.00% 2 Missing ⚠️
src/codegen/codegen_acc_visitor.cpp 0.00% 1 Missing ⚠️
src/codegen/codegen_acc_visitor.hpp 50.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1078      +/-   ##
==========================================
- Coverage   88.67%   88.18%   -0.50%     
==========================================
  Files         170      175       +5     
  Lines       12540    12903     +363     
==========================================
+ Hits        11120    11378     +258     
- Misses       1420     1525     +105

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot

This comment has been minimized.

Sign in to view
@iomaganaris
Renamed CodegenCppVisitor to CodegenCoreneuronCppVisitor
2ba8a68
@iomaganaris
Added NEURON code generator
992c155
@iomaganaris
Create CodegenCppVisitor and use common contructor for CodegenCoreneu…
fe6d3a8 
…ronCppVisitor and CodegenNeuronCppVisitor
@iomaganaris
Make the CodegenCppVisitor constructors protected
ec1996d
@iomaganaris
Initial commit with working distinction between Codegen visitors for …
cb5c267 
…NEURON and CoreNEURON using CodegenCppVisitor
@iomaganaris
Cleaned a bit the CodegenNeuronCppVisitor to generate a cleaner C++ f…
0085ce7 
…ile and added comment regarding good TODOs
@iomaganaris
Generate a cleaner C++ file for NEURON
56694ef
@iomaganaris
Move few common functions in CodegenCppVisitor
3f83ea9
@iomaganaris
Remove wrapper routing printing function
bea8b97
@iomaganaris
Moving some member variables from codegen(core)neuroncppvisitor to co…
5635409 
…degencppvisitor
@iomaganaris
Run formatting tool
f7ed94e
@iomaganaris
Some initial changes in NEURON boilerplate
88551ac
@iomaganaris
Move variable name printing across neuron and coreneuron codegen & ot…
93fb783 
…her small changes
@iomaganaris
Fix instantiation of array state variables
54bef9b
@iomaganaris
Update includes and using declarations
e011083
@iomaganaris
Fix printing of float variable macros
3ee01c8
@iomaganaris iomaganaris marked this pull request as ready for review October 17, 2023 15:00
@iomaganaris
Copy link
Contributor Author

Failing CI is related to #1089

@bbpbuildbot

This comment has been minimized.

Sign in to view
ohm314
ohm314 reviewed Oct 17, 2023
View reviewed changes
Copy link
Contributor

@ohm314 ohm314 left a comment

Choose a reason for hiding this comment

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

I didn't check the (coreneuron|neuron)_cpp_codegen_visitor.(cpp|hpp) files in full detail since it's mostly copying around of code, which plaid havoc with github's diff but I looked over the lest, which seems very reasonable. I can look in more detail at the codegen visitors in a next iteration if it makes sense!

src/codegen/codegen_acc_visitor.cpp Show resolved Hide resolved
src/main.cpp Outdated Show resolved Hide resolved
src/main.cpp Outdated Show resolved Hide resolved
test/unit/visitor/sympy_solver.cpp Show resolved Hide resolved
@bbpbuildbot

This comment has been minimized.

Sign in to view
pramodk
pramodk reviewed Oct 20, 2023
View reviewed changes
Copy link
Contributor

@pramodk pramodk left a comment

Choose a reason for hiding this comment

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

@iomaganaris : few minor comments and some clarifications. Overall seems good to me and I agree that it would be good to get this in to avoid this remaining in a branch for a long time.

test/unit/codegen/codegen_neuron_cpp_visitor.cpp Outdated Show resolved Hide resolved
src/codegen/codegen_info.hpp Show resolved Hide resolved
src/main.cpp Outdated Show resolved Hide resolved
src/codegen/codegen_cpp_visitor.hpp Show resolved Hide resolved
src/codegen/codegen_coreneuron_cpp_visitor.hpp Show resolved Hide resolved
src/codegen/codegen_cpp_visitor.hpp Show resolved Hide resolved
src/codegen/codegen_neuron_cpp_visitor.cpp Show resolved Hide resolved
src/codegen/codegen_neuron_cpp_visitor.cpp Show resolved Hide resolved
src/codegen/codegen_neuron_cpp_visitor.cpp Show resolved Hide resolved
src/codegen/codegen_neuron_cpp_visitor.cpp Outdated Show resolved Hide resolved
@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot

This comment has been minimized.

Sign in to view
ohm314
ohm314 reviewed Nov 28, 2023
View reviewed changes
src/main.cpp Outdated Show resolved Hide resolved
ohm314
ohm314 approved these changes Nov 28, 2023
View reviewed changes
Copy link
Contributor

@ohm314 ohm314 left a comment

Choose a reason for hiding this comment

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

I think this is in good shape!

@bbpbuildbot

This comment has been minimized.

Sign in to view
@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #175355 (:white_check_mark:) have been uploaded here!

Status and direct links:

@iomaganaris iomaganaris merged commit 2c4c8e2 into master Nov 29, 2023
16 of 18 checks passed
@iomaganaris iomaganaris deleted the neuron_nmodl_2 branch November 29, 2023 13:24
@iomaganaris iomaganaris added the NEURON codegen Work toward NEURON code generation label Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pramodk pramodk pramodk left review comments

@ohm314 ohm314 ohm314 approved these changes

@tristan0x tristan0x Awaiting requested review from tristan0x

@alkino alkino Awaiting requested review from alkino

Assignees
No one assigned
Labels
NEURON codegen Work toward NEURON code generation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@iomaganaris @bbpbuildbot @ohm314 @pramodk