NEURON
Todo List
Member coreneuron::nrn_cleanup_ion_map ()
coreneuron should have finalise callback which can be called from NEURON for final memory cleanup including global state like registered mechanisms and ions map.
Member coreneuron::nrnran123_newstream3 (uint32_t id1, uint32_t id2, uint32_t id3, bool use_unified_memory)
It would be nicer if the API return type was std::unique_ptr<nrnran123_State, ...not specified...>, so we could use a custom allocator/deleter and avoid the (fragile) need for matching nrnran123_deletestream calls.
Member corenrn_embedded_run (int nthread, int have_gaps, int use_mpi, int use_fast_imem, const char *mpi_lib, const char *nrn_arg, int file_mode)
Change return type semantics
Member hoc_get_arg (std::size_t narg)
Should the stack be modified such that this can return const references, even for things like data_handle<T> that at the moment are not exactly stored (we store generic_data_handle, which can produce a data_handle<T> on demand but which does not, at present, actually contain a data_handle<T>)?
Member Memb_list::set_storage_offset (std::size_t offset)
At the moment this is set as part of sorting/permuting data, but it is not automatically invalidated when the cache / sorted status is reset. Consider if these offsets can be more explicitly tied to the lifetime of the cache data.
File modl.h
Add these bit masks as enum-flags and remove this legacy header
Class neuron::container::data_handle< T >
Const correctness – data_handle should be like span: data_handle<double> can read + write the value, data_handle<double const> can only read the value. const applied to the data_handle itself should just control whether or not it can be rebound to refer elsewhere.
Member neuron::container::detail::field_data< Tag, FieldImplementation::RuntimeVariable >::m_array_dim_prefix_sums
This could be used to more efficiently convert legacy indices.
Class neuron::container::generic_data_handle
Consider whether this should be made more like std::any (with a maximum 2*sizeof(void*) and a promise never to allocate memory dynamically) so it actually has a data_handle<T> subobject. Presumably that would mean data_handle<T> would need to have a trivial destructor. This might make it harder in future to have some vector_of_generic_data_handle type that hoists out the pointer-to-container and typeid parts that should be the same for all rows.
Member neuron::container::generic_data_handle::get () const
Consider conversion to bool and whether this means not-null or to obtain a literal, wrapped bool value
Member neuron::container::handle_base< Identifier >::get_handle ()
const cleanup – should there be a const version returning data_handle<T const>?
Member neuron::container::handle_base< Identifier >::get_handle (int field_index, int array_offset=0)
Const cleanup as above for the zero-argument version.
Member neuron::container::handle_base< Identifier >::id_hack () const
Remove those macros once and for all.
Class neuron::container::Mechanism::field::FloatingPoint
Update the code generation so we get some hh_data = soa<hh_identifier, hh_a, hh_b, ...> type instead of fudging things this way.
Member neuron::container::Node::handle_interface< Identifier >::area_hack ()
Remove those macros once and for all.
Member neuron::container::Node::handle_interface< Identifier >::area_hack () const
Remove those macros once and for all.
Member neuron::container::Node::handle_interface< Identifier >::v_hack ()
Remove those macros once and for all.
Member neuron::container::Node::handle_interface< Identifier >::v_hack () const
Remove those macros once and for all.
Member neuron::container::soa< Storage, Tags >::find_data_handle (neuron::container::generic_data_handle input_handle) const
Check const-correctness. Presumably a const version would return data_handle<T const>, which would hold a pointer-to-const for the container?
Member neuron::container::soa< Storage, Tags >::is_storage_pointer (typename Tag::type const *ptr) const
Fix this for tag types with num_variables()?
Member neuron::container::soa< Storage, Tags >::issue_frozen_token ()
A future extension could be to preserve the sorted flag until pointers are actually, not potentially, invalidated.
Class nmodl::ast::Ast
With the ast::Node as another top level node, this can be removed in the future.
Class nmodl::ast::ConstantStatement
As ConstantStatement wraps a single ConstantVar, this or ast::ConstantVar can be redundant in the future.
Member nmodl::codegen::CodegenAccVisitor::print_abort_routine () const override
: we need to implement proper error handling mechanism to propogate errors from GPU to CPU. For example, error code can be returned like original neuron implementation. For now we use assert(0==1) pattern which is used for OpenACC.
Member nmodl::codegen::CodegenAccVisitor::print_net_init_acc_serial_annotation_block_begin () override
: With the current code structure for NMODL and MOD2C, we use serial construct to launch serial kernels. This is during initialization but still inefficient. This should be improved when we drop MOD2C.
Class nmodl::codegen::CodegenCoreneuronCppVisitor
  • Handle define statement (i.e. macros)
  • If there is a return statement in the verbatim block of inlined function then it will be error. Need better error checking. For example, see netstim.mod where we have removed return from verbatim block.
Member nmodl::codegen::CodegenCoreneuronCppVisitor::append_conc_write_statements (std::vector< ShadowUseStatement > &statements, const Ion &ion, const std::string &concentration) override
Unhandled case in neuron implementation
Member nmodl::codegen::CodegenCoreneuronCppVisitor::internal_method_parameters () override
: figure out how to correctly handle qualifiers
Member nmodl::codegen::CodegenCoreneuronCppVisitor::print_derivimplicit_kernel (const ast::Block &block)
Data is not derived.
Member nmodl::codegen::CodegenCoreneuronCppVisitor::print_nrn_state () override
Eigen solver node also emits IonCurVar variable in the functor but that shouldn't update ions in derivative block
Member nmodl::codegen::CodegenCoreneuronCppVisitor::print_watch_activate ()

Similar to neuron/coreneuron we are using first watch and ignoring rest.

Number of watch could be more than number of statements according to grammar.

Member nmodl::codegen::CodegenCoreneuronCppVisitor::print_watch_check ()
Similar to print_watch_activate, we are using only first watch.
Member nmodl::codegen::CodegenCoreneuronCppVisitor::process_verbatim_text (std::string const &text)
: this is still ad-hoc and requires re-implementation to handle it more elegantly.
Class nmodl::codegen::CodegenCppVisitor
  • Handle define statement (i.e. macros)
  • If there is a return statement in the verbatim block of inlined function then it will be error. Need better error checking. For example, see netstim.mod where we have removed return from verbatim block.
Member nmodl::codegen::CodegenCppVisitor::codegen_global_variables
: this has become different than CodegenInfo
Member nmodl::codegen::CodegenCppVisitor::visit_var_name (const ast::VarName &node) override
: Validate how @ is being handled in neuron implementation
Class nmodl::codegen::CodegenHelperVisitor
  • Determine vectorize as part of the pass
  • Determine threadsafe as part of the pass
  • Global variable order is not preserved, for example, below gives different order:
    • NEURON block: GLOBAL gq, gp
    • PARAMETER block: gp = 11, gq[2]
  • POINTER rng and if it's also assigned rng[4] then it is printed as one value. Need to check what is correct value.
Member nmodl::codegen::CodegenHelperVisitor::find_non_range_variables ()
Below we calculate thread related id and sizes. This will need to do from global analysis pass as here we are handling top local variables, global variables, derivimplicit method. There might be more use cases with other solver methods.
Member nmodl::codegen::CodegenHelperVisitor::visit_statement_block (const ast::StatementBlock &node) override
AST can have duplicate DERIVATIVE blocks if a mod file uses SOLVE statements in its INITIAL block (e.g. in case of kinetic schemes using STEADYSTATE sparse solver). Such duplicated DERIVATIVE blocks could be removed by SolveBlockVisitor, or we have to avoid visiting them here. See e.g. SH_na8st.mod test and original reduced_dentate .mod.
Class nmodl::codegen::CodegenInfo
Need to store all Define i.e. macro definitions?
Class nmodl::codegen::CodegenNeuronCppVisitor
  • Handle define statement (i.e. macros)
  • If there is a return statement in the verbatim block of inlined function then it will be error. Need better error checking. For example, see netstim.mod where we have removed return from verbatim block.
Member nmodl::codegen::CodegenNeuronCppVisitor::print_nrn_state () override
Eigen solver node also emits IonCurVar variable in the functor but that shouldn't update ions in derivative block
Member nmodl::codegen::naming::NMODL_VERSION []
: should be moved from codegen to global scope
Class nmodl::codegen::ShadowUseStatement
If shadow_lhs is empty then we assume shadow statement not required
Member nmodl::details::keywords
Some keywords have different token names, e.g. TITLE keyword has MODEL as a keyword. These token names are used in multiple context and hence we are keeping original names. Once we finish code generation part then we change this.
Member nmodl::details::methods
MethodInfo::subtype should be changed from integer flag to proper type
Class nmodl::ModToken
  • LocationType object is copyable except if we specify the stream name. It would be good to track filename when we go for multi-channel optimization and code generation.
Member nmodl::name_symbol (const std::string &text, PositionType &pos, TokenType type)
In addition to keywords and methods, there are also external definitions for math and neuron specific functions/variables. In the token we should mark those as external.
Namespace nmodl::parser::diffeq
The implementations here are verbose and has duplicate code. Need to revisit this, may be using better library like symengine altogether.
Member nmodl::parser::diffeq::DiffEqContext::cvode_deriv () const
Methods inherited neuron implementation
Member nmodl::parser::diffeq::DiffEqContext::get_solution (bool &cnexp_possible)
Currently we have tested cnexp, euler and derivimplicit methods with all equations from BBP models. Need to test this against various other mod files, especially kinetic schemes, reaction-diffusion etc.
Member nmodl::parser::diffeq::Term::a
Need to check in neuron implementation?
Member nmodl::parser::DiffeqDriver::cnexp_possible (const std::string &equation, std::string &solution)
Instead of using neuron like api, we need to refactor
Class nmodl::parser::NmodlDriver

Lexer, parser and ast member variables are used inside lexer/ parser instances. The local instaces are created inside parse_stream and hence the pointers are no longer valid except ast. Need better way to handle this.

Stream name is not used as it will need better support as location object used in scanner takes string pointer which could be invalid when we copy location object.

Class nmodl::printer::JSONPrinter
We need to explicitly call flush() in order to get write/return results. We simply can't dump block in popBlock() because block itself will be part of other parent elements. Also we are writing results to file, stringstream and cout. And hence we can't simply reset/clear previously written text.
Class nmodl::printer::NMODLPrinter
Implement Printer as base class to avoid duplication code between JSONPrinter and NMODLPrinter.
Class nmodl::symtab::ModelSymbolTable
Unique name should be based on location. Use ModToken to get position.
Member nmodl::symtab::ModelSymbolTable::get_unique_name (const std::string &name, ast::Ast *node, bool is_global)
We should add position information to make name unique
Class nmodl::symtab::Symbol
  • Multiple tokens (i.e. location information) for symbol should be tracked
  • Scope information should be more than just string
  • Perf block should track information about all usage of the symbol (would be helpful for perf modeling)
  • Need to keep track of all renaming information, currently only we keep last state
Member nmodl::symtab::Symbol::is_external_variable () const noexcept
Need to check if we should check two properties using has_any_property instead of exact comparison
Class nmodl::symtab::SymbolTable
  • Revisit when clone method is used and implementation of copy constructor
  • Name may not require as we have added AST node
Member nmodl::symtab::SymbolTable::clone () const
Revisit the usage as tokens will be pointing to old nodes
Class nmodl::symtab::SymbolTable::Table
Re-implement pretty printing
Member nmodl::symtab::syminfo::enum_type
Error with pybind if std::underlying_typ is used
Member nmodl::symtab::syminfo::NmodlType
  • Rename param_assign to parameter_var
Member nmodl::test_utils::NmodlTestCase::NmodlTestCase ()=delete
: add associated json (to use in visitor test)
Member nmodl::visitor::create_statement (const std::string &code_statement)
Need to revisit this during code generation passes to make sure if all statements can be part of procedure block.
Member nmodl::visitor::DefUseAnalyzeVisitor::visit_verbatim (const ast::Verbatim &node) override
One simple way would be to look for p_name in the string of verbatim block to find the variable usage.
Class nmodl::visitor::InlineVisitor
  • Recursive function calls are not supported and need to add checks to avoid stack explosion
  • Currently we rename variables more than necessary, this could be improved [low priority]
  • Function calls as part of an argument of function call itself are not completely inlined [low priority]
  • Symbol table pass needs to be re-run in order to update the definitions/usage
  • Location of symbol/nodes after inlining still points to old nodes
Member nmodl::visitor::InlineVisitor::can_replace_statement (const std::shared_ptr< ast::Statement > &statement)
Add method to ast itself to simplify this implementation
Class nmodl::visitor::LocalizeVisitor
  • We are excluding procedures/functions because they will be still using global variables. We need to have dead-code removal pass to eliminate unused procedures/ functions before localizer pass.
Member nmodl::visitor::LocalizeVisitor::variables_to_optimize () const

Voltage v can be global variable as well as external. In order to avoid optimizations, we need to handle this case properly

Instead of ast node, use symbol properties to check variable type

Class nmodl::visitor::LocalToAssignedVisitor
  • Variables like qt are often constant. As long as INITIAL block is executed serially or qt is updated in atomic way then we don't have a problem.
Class nmodl::visitor::LocalVarRenameVisitor
  • Currently we are renaming variables even if there is no inlining candidates. In this case ideally we should not rename.
Class nmodl::visitor::PerfVisitor
  • To measure the performance of statements like if, elseif and else, we have to find maximum performance from if,elseif,else and then use it to calculate total performance. In the current implementation we are doing sum of all blocks. We need to override IfStatement (which has all sub-blocks) and get maximum performance of all statements recursively.
Class nmodl::visitor::RenameVisitor
Add log/warning messages.
Member nmodl::visitor::SymtabVisitor::visit_table_statement (ast::TableStatement &node) override
we assume table statement follows variable declaration
Class nmodl::visitor::VarUsageVisitor
Check if macro is considered as variable
Class nmodl::visitor::VerbatimVarRenameVisitor
Check if symbol table lookup is ok or there are cases where this could be error prone.
Member Prop::translate_legacy_index (int legacy_index) const
Reimplement this using the new helpers.
Member QTYPE
: check if stl queue works with move_event functions.
Member SCENARIO ("Global symbol table (ModelSymbol) allows scope based operations")
: not sure how to capture std::cout