Include compiled mod files. #1131

1uc · 2023-12-20T09:04:22Z

Add a script for generating the reference values of compiled mod files. These are included with the sources to enable the following:

during review the reviewer can see what was generated.
these can serve as golden tests, flagging changes in unrelated mod files.

test/usecases/cnexp_scalar/leonhard.cpp

1uc · 2023-12-20T09:14:17Z

The hope is that it can generate a suite of references. Then when things change we have diff to print exactly which lines changed, probably we can get it to print which characters changed too.

The advantage is that we can automatically update the references; and don't need to do so manually. The cost is that we need to commit entire files. Meaning we test the whole file not just the "interesting" parts of each usecase. Which means we're sensitive to more changes.

codecov · 2023-12-20T09:50:08Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.40%. Comparing base (9df433b) to head (b13a0a3).
Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1131      +/-   ##
==========================================
+ Coverage   88.38%   88.40%   +0.02%     
==========================================
  Files         176      175       -1     
  Lines       12946    12934      -12     
==========================================
- Hits        11442    11434       -8     
+ Misses       1504     1500       -4

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

iomaganaris

Those "golden" reference files could be a potential solution. However I think that this PR is missing a test that compares the "golden" reference file with the one generated using the NMODL being tested.
Let's discuss this with @pramodk and @ohm314 to see what Omar has been working on to make the diff of these "golden" reference files nicer and see how to proceed

1uc · 2024-01-10T07:56:01Z

Yes, I agree. In a first step I'd like to understand if it helps with my difficulty reviewing PRs. While I can see what changed and might understand at a high-level what's happening, I can't see the generated code. The question is: does seeing how the generated code changed help understand a PR?

I suspect this might help with that. As for checking/diffing: fist time they're uploaded the references are completely new. After that the diff will only contain the lines that change due to a specific PR. Preferably one would add the file in a separate commit. Before any changes. Then make changes and have the diff already valid on the first PR.

iomaganaris · 2024-01-10T16:52:43Z

The question is: does seeing how the generated code changed help understand a PR?

It definitely helps 👍

After that the diff will only contain the lines that change due to a specific PR. Preferably one would add the file in a separate commit. Before any changes. Then make changes and have the diff already valid on the first PR.

I am afraid that with so many files we are going to start forgetting to update them in case there are any changes or miss small changes to them. I believe it's necessary to have a way to automatically update them when opening a PR with some changes. Once way to enforce that is to have a test that fails if there is a difference between the generated and the "golden" file.

1uc · 2024-01-10T18:09:24Z

I am afraid that with so many files we are going to start forgetting to update them in case there are any changes or miss small changes to them.

100% correct, that's an important part missing: those files must be automatically generated.

pramodk · 2024-02-09T11:08:58Z

@1uc : I assume this will be updated by pushing references to https://github.com/BlueBrain/nmodl-references. So I will keep this as it is.

1uc · 2024-02-09T12:44:17Z

Yes, exactly.

1uc · 2024-02-20T15:17:46Z

@pramodk the generated files have been move to the submodule. there's a PR ready in the submodule:
BlueBrain/nmodl-references#1

1uc · 2024-02-22T09:05:42Z

Finally: https://github.com/BlueBrain/nmodl/actions/runs/8001921784/job/21854083478?pr=1131#step:15:245

1uc · 2024-02-22T15:02:36Z

I currently have a different PR which introduces changes. To help other judge if this is useful, I'll post the output of the two approaches: unit-testing and external as sketched here.

1uc · 2024-02-22T15:03:55Z

The proposed tests fail as follows:

$ ctest --test-dir TEST_DIR --output-on-failure
...
[NMODL] [info] :: Running C++ backend code generator for CoreNEURON
diff -r /home/lucg/git/bbp/nmodl/test/usecases/references/func_proc_pnt/neuron/func_proc_pnt.cpp /tmp/tmp.PQmpGOAFU8/neuron/func_proc_pnt.cpp
84a85,87
>         int const * _nodeindices;
>         double const * _node_voltages;
>         double * _node_rhs;
92c95
<     static test_func_proc_pnt_Instance make_instance_test_func_proc_pnt(_nrn_mechanism_cache_range& _ml) {
---
>     static test_func_proc_pnt_Instance make_instance_test_func_proc_pnt(_nrn_mechanism_cache_range& _ml, NrnThread& _nt, Memb_list& _ml_arg) {
93a97,99
>             _ml_arg.nodeindices,
>             _nt.node_voltage_storage(),
>             _nt.node_rhs_storage(),
146,148c152,154
<     inline double get_a_42_test_func_proc_pnt(_nrn_mechanism_cache_range* _ml, size_t id, Datum* _ppvar, Datum* _thread, NrnThread* _nt, double a);
<     inline int set_x_42_test_func_proc_pnt(_nrn_mechanism_cache_range* _ml, size_t id, Datum* _ppvar, Datum* _thread, NrnThread* _nt);
<     inline int set_x_a_test_func_proc_pnt(_nrn_mechanism_cache_range* _ml, size_t id, Datum* _ppvar, Datum* _thread, NrnThread* _nt, double a);
---
>     inline double get_a_42_test_func_proc_pnt(size_t id, test_func_proc_pnt_Instance& inst, double v, double a);
>     inline int set_x_42_test_func_proc_pnt(size_t id, test_func_proc_pnt_Instance& inst, double v);
>     inline int set_x_a_test_func_proc_pnt(size_t id, test_func_proc_pnt_Instance& inst, double v, double a);
199c205
<         set_x_42_test_func_proc_pnt(_ml, id, _ppvar, _thread, _nt);
---
>         set_x_42_test_func_proc_pnt(id, inst, v);
219c225
<         set_x_a_test_func_proc_pnt(_ml, id, _ppvar, _thread, _nt, *getarg(1));
---
>         set_x_a_test_func_proc_pnt(id, inst, v, *getarg(1));
238c244
<         _r = get_a_42_test_func_proc_pnt(_ml, id, _ppvar, _thread, _nt, *getarg(1));
---
>         _r = get_a_42_test_func_proc_pnt(id, inst, v, *getarg(1));
243,244c249,250
<     inline int set_x_42_test_func_proc_pnt(_nrn_mechanism_cache_range* _ml, size_t id, Datum* _ppvar, Datum* _thread, NrnThread* _nt) {
<         auto inst = make_instance_test_func_proc_pnt(*_ml);
---
>     inline int set_x_42_test_func_proc_pnt(size_t id, test_func_proc_pnt_Instance& inst, double v) {
>         auto inst = make_instance_test_func_proc_pnt(*_ml, *_nt, *_ml_arg);
251,252c257,258
<     inline int set_x_a_test_func_proc_pnt(_nrn_mechanism_cache_range* _ml, size_t id, Datum* _ppvar, Datum* _thread, NrnThread* _nt, double a) {
<         auto inst = make_instance_test_func_proc_pnt(*_ml);
---
>     inline int set_x_a_test_func_proc_pnt(size_t id, test_func_proc_pnt_Instance& inst, double v, double a) {
>         auto inst = make_instance_test_func_proc_pnt(*_ml, *_nt, *_ml_arg);
259,260c265,266
<     inline double get_a_42_test_func_proc_pnt(_nrn_mechanism_cache_range* _ml, size_t id, Datum* _ppvar, Datum* _thread, NrnThread* _nt, double a) {
<         auto inst = make_instance_test_func_proc_pnt(*_ml);
---
>     inline double get_a_42_test_func_proc_pnt(size_t id, test_func_proc_pnt_Instance& inst, double v, double a) {
>         auto inst = make_instance_test_func_proc_pnt(*_ml, *_nt, *_ml_arg);
269c275
<         auto inst = make_instance_test_func_proc_pnt(_lmr);
---
>         auto inst = make_instance_test_func_proc_pnt(_lmr, *_nt, *_ml_arg);

1uc · 2024-02-22T15:05:26Z

The unit-test fail as follows:

 18/126 Test  #18: testcodegen/Scenario: Check NEURON codegen for simple MOD file ......................................***Failed    0.03 sec
Filters: "Scenario: Check NEURON codegen for simple MOD file"
Randomness seeded to: 3724744856

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
testcodegen is a Catch2 v3.3.2 host application.
Run with -? for options

-------------------------------------------------------------------------------
Scenario: Check NEURON codegen for simple MOD file
      Given: A simple mod file with RANGE, ARRAY and ION variables
       Then: Correct pas_test_Instance
-------------------------------------------------------------------------------
/home/lucg/git/bbp/nmodl/test/unit/codegen/codegen_neuron_cpp_visitor.cpp:186
...............................................................................

/home/lucg/git/bbp/nmodl/test/unit/codegen/codegen_neuron_cpp_visitor.cpp:205: FAILED:
  REQUIRE_THAT( generated, ContainsSubstring(reindent_and_trim_text(expected)) )
with expansion:
  "/*********************************************************
  Model Name      : pas_test
  Filename        : _test.mod
  NMODL Version   : 7.7.0
  Vectorized      : true
  Threadsafe      : true
  Created         : Thu Feb 22 15:59:48 2024
  Simulator       : NEURON
  Backend         : C++ (api-compatibility)
  NMODL Compiler  : 0.6 [eb3215b7 2024-02-22 15:59:27 +0100]
  *********************************************************/
  
#include <math.h>
  #include <stdio.h>
  #include <stdlib.h>
  
#include "mech_api.h"
  #include "neuron/cache/mechanism_range.hpp"
  #include "nrniv_mf.h"
  #include "section_fwd.hpp"
  
/* NEURON global macro definitions */
  /* VECTORIZED */
  #define NRN_VECTORIZED 1
  
static constexpr auto number_of_datum_variables = 3;
  static constexpr auto number_of_floating_point_variables = 10;
  
namespace {
  template <typename T>
  using _nrn_mechanism_std_vector = std::vector<T>;
  using _nrn_model_sorted_token = neuron::model_sorted_token;
  using _nrn_mechanism_cache_range = neuron::cache::MechanismRange
  <number_of_floating_point_variables, number_of_datum_variables>;
  using _nrn_mechanism_cache_instance = neuron::cache::MechanismInstance
  <number_of_floating_point_variables, number_of_datum_variables>;
  using _nrn_non_owning_id_without_container = neuron::container::
  non_owning_identifier_without_container;
  template <typename T>
  using _nrn_mechanism_field = neuron::mechanism::field<T>;
  template <typename... Args>
  void _nrn_mechanism_register_data_fields(Args&&... args) {
      neuron::mechanism::register_data_fields(std::forward<Args>(args)...);
  }
  }  // namespace
  
Prop* hoc_getdata_range(int type);
  

  namespace neuron {
      #ifndef NRN_PRCELLSTATE
      #define NRN_PRCELLSTATE 0
      #endif
  

      /** channel information */
      static const char *mechanism_info[] = {
          "7.7.0",
          "pas_test",
          "g_pas_test",
          "e_pas_test",
          0,
          "i_pas_test",
          "ar_pas_test[2]",
          0,
          "s_pas_test",
          0,
          0
      };
  

      /* NEURON global variables */
      static neuron::container::field_index _slist1[1], _dlist1[1];
      static Symbol* _na_sym;
      static int mech_type;
      static Prop* _extcall_prop;
      /* _prop_id kind of shadows _extcall_prop to allow validity checking. */
      static _nrn_non_owning_id_without_container _prop_id{};
      static int hoc_nrnpointerindex = -1;
      static _nrn_mechanism_std_vector<Datum> _extcall_thread;
  

      /** all global variables */
      struct pas_test_Store {
          double s0{};
      };
      static_assert(std::is_trivially_copy_constructible_v<pas_test_Store>);
      static_assert(std::is_trivially_move_constructible_v<pas_test_Store>);
      static_assert(std::is_trivially_copy_assignable_v<pas_test_Store>);
      static_assert(std::is_trivially_move_assignable_v<pas_test_Store>);
      static_assert(std::is_trivially_destructible_v<pas_test_Store>);
      pas_test_Store pas_test_global;
  

      /** all mechanism instance variables and global variables */
      struct pas_test_Instance  {
          int const * _nodeindices;
          double const * _node_voltages;
          double * _node_rhs;
          double* g{};
          double* e{};
          double* i{};
          double* ar{};
          double* s{};
          double* ena{};
          double* ina{};
          double* Ds{};
          double* v_unused{};
          double* g_unused{};
          const double* const* ion_ena{};
          double* const* ion_ina{};
          double* const* ion_dinadv{};
          pas_test_Store* global{&pas_test_global};
      };
  

      static pas_test_Instance make_instance_pas_test
  (_nrn_mechanism_cache_range& _ml, NrnThread& _nt, Memb_list& _ml_arg) {
          return pas_test_Instance {
              _ml_arg.nodeindices,
              _nt.node_voltage_storage(),
              _nt.node_rhs_storage(),
              _ml.template fpfield_ptr<0>(),
              _ml.template fpfield_ptr<1>(),
              _ml.template fpfield_ptr<2>(),
              _ml.template data_array_ptr<3, 2>(),
              _ml.template fpfield_ptr<4>(),
              _ml.template fpfield_ptr<5>(),
              _ml.template fpfield_ptr<6>(),
              _ml.template fpfield_ptr<7>(),
              _ml.template fpfield_ptr<8>(),
              _ml.template fpfield_ptr<9>(),
              _ml.template dptr_field_ptr<0>(),
              _ml.template dptr_field_ptr<1>(),
              _ml.template dptr_field_ptr<2>()
          };
      }
  

      static void nrn_alloc_pas_test(Prop* _prop) {
          Prop *prop_ion{};
          Datum *_ppvar{};
          _ppvar = nrn_prop_datum_alloc(mech_type, 3, _prop);
          _nrn_mechanism_access_dparam(_prop) = _ppvar;
          _nrn_mechanism_cache_instance _ml_real{_prop};
          auto* const _ml = &_ml_real;
          size_t const _iml{};
          assert(_nrn_mechanism_get_num_vars(_prop) == 10);
          /*initialize range parameters*/
          _ml->template fpfield<0>(_iml) = 0.001; /* g */
          _ml->template fpfield<1>(_iml) = -70; /* e */
          _nrn_mechanism_access_dparam(_prop) = _ppvar;
          Symbol * na_sym = hoc_lookup("na_ion");
          Prop * na_prop = need_memb(na_sym);
          _ppvar[0] = _nrn_mechanism_get_param_handle(na_prop, 0);
          _ppvar[1] = _nrn_mechanism_get_param_handle(na_prop, 3);
      }
  

      /* Neuron setdata functions */
      extern void _nrn_setdata_reg(int, void(*)(Prop*));
      static void _setdata(Prop* _prop) {
          _extcall_prop = _prop;
          _prop_id = _nrn_get_prop_id(_prop);
      }
      static void _hoc_setdata() {
          Prop *_prop = hoc_getdata_range(mech_type);
          _setdata(_prop);
          hoc_retpushx(1.);
      }
      /* Mechanism procedures and functions */
  

      /** connect global (scalar) variables to hoc -- */
      static DoubScal hoc_scalar_double[] = {
          {nullptr, nullptr}
      };
  

      /** connect global (array) variables to hoc -- */
      static DoubVec hoc_vector_double[] = {
          {nullptr, nullptr, 0}
      };
  

      /* declaration of user functions */
  

      /* connect user functions to hoc names */
      static VoidFunc hoc_intfunc[] = {
          {"setdata_pas_test", _hoc_setdata},
          {0, 0}
      };
      static NPyDirectMechFunc npy_direct_func_proc[] = {
      };
  

      void nrn_init_pas_test(_nrn_model_sorted_token const& _sorted_token,
  NrnThread* _nt, Memb_list* _ml_arg, int _type) {
          _nrn_mechanism_cache_range _lmr{_sorted_token, *_nt, *_ml_arg, _type}
  ;
          auto inst = make_instance_pas_test(_lmr, *_nt, *_ml_arg);
          auto nodecount = _ml_arg->nodecount;
          for (int id = 0; id < nodecount; id++) {
              inst.ena[id] = (*inst.ion_ena[id]);
              (inst.ar+id*2)[static_cast<int>(0)] = 1.0;
          }
      }
  

      inline double nrn_current_pas_test(size_t id, pas_test_Instance& inst,
  double v) {
          double current = 0.0;
          inst.i[id] = inst.g[id] * (v - inst.e[id]);
          inst.ina[id] = inst.g[id] * (v - inst.ena[id]);
          current += inst.i[id];
          current += inst.ina[id];
          return current;
      }
  

      /** update current */
      void nrn_cur_pas_test(_nrn_model_sorted_token const& _sorted_token,
  NrnThread* _nt, Memb_list* _ml_arg, int _type) {
          _nrn_mechanism_cache_range _lmr{_sorted_token, *_nt, *_ml_arg, _type}
  ;
          auto inst = make_instance_pas_test(_lmr, *_nt, *_ml_arg);
          auto nodecount = _ml_arg->nodecount;
          for (int id = 0; id < nodecount; id++) {
              int node_id = inst._nodeindices[id];
              double v = inst._node_voltages[node_id];
              inst.ena[id] = (*inst.ion_ena[id]);
              double I1 = nrn_current_pas_test(id, inst, v+0.001);
              double dina = inst.ina[id];
              double I0 = nrn_current_pas_test(id, inst, v);
              double rhs = I0;
              double g = (I1-I0)/0.001;
              (*inst.ion_dinadv[id]) += (dina-inst.ina[id])/0.001;
              (*inst.ion_ina[id]) += inst.ina[id];
              inst._node_rhs[node_id] -= rhs;
          }
      }
  

      void nrn_state_pas_test(_nrn_model_sorted_token const& _sorted_token,
  NrnThread* _nt, Memb_list* _ml_arg, int _type) {
          _nrn_mechanism_cache_range _lmr{_sorted_token, *_nt, *_ml_arg, _type}
  ;
          auto inst = make_instance_pas_test(_lmr, *_nt, *_ml_arg);
          auto nodecount = _ml_arg->nodecount;
          for (int id = 0; id < nodecount; id++) {
              inst.ena[id] = (*inst.ion_ena[id]);
              inst.s[id] = inst.s[id] - _nt->_dt * ( -((inst.ar+id*2)
  [static_cast<int>(0)]));
          }
      }
  

      /** nrn_jacob function */
      static void nrn_jacob_pas_test(_nrn_model_sorted_token const&
  _sorted_token, NrnThread* _nt, Memb_list* _ml_arg, int _type) {}
  

      static void _initlists() {
          /* s */
          _slist1[0] = {4, 0};
          /* Ds */
          _dlist1[0] = {7, 0};
      }
  

      /** register channel with the simulator */
      extern "C" void __test_reg() {
          _initlists();
  
        ion_reg("na", -10000.);
  
        _na_sym = hoc_lookup("na_ion");
  
        register_mech(mechanism_info, nrn_alloc_pas_test, nrn_cur_pas_test,
  nrn_jacob_pas_test, nrn_state_pas_test, nrn_init_pas_test,
  hoc_nrnpointerindex, 1);
  
        mech_type = nrn_get_mechtype(mechanism_info[1]);
          _nrn_mechanism_register_data_fields(mech_type,
              _nrn_mechanism_field<double>{"g"} /* 0 */,
              _nrn_mechanism_field<double>{"e"} /* 1 */,
              _nrn_mechanism_field<double>{"i"} /* 2 */,
              _nrn_mechanism_field<double>{"ar", 2} /* 3 */,
              _nrn_mechanism_field<double>{"s"} /* 4 */,
              _nrn_mechanism_field<double>{"ena"} /* 5 */,
              _nrn_mechanism_field<double>{"ina"} /* 6 */,
              _nrn_mechanism_field<double>{"Ds"} /* 7 */,
              _nrn_mechanism_field<double>{"v_unused"} /* 8 */,
              _nrn_mechanism_field<double>{"g_unused"} /* 9 */,
              _nrn_mechanism_field<double*>{"ion_ena", "na_ion"} /* 0 */,
              _nrn_mechanism_field<double*>{"ion_ina", "na_ion"} /* 1 */,
              _nrn_mechanism_field<double*>{"ion_dinadv", "na_ion"} /* 2 */
          );
  
        hoc_register_prop_size(mech_type, 10, 3);
          hoc_register_dparam_semantics(mech_type, 0, "na_ion");
          hoc_register_dparam_semantics(mech_type, 1, "na_ion");
          hoc_register_dparam_semantics(mech_type, 2, "na_ion");
          hoc_register_var(hoc_scalar_double, hoc_vector_double, hoc_intfunc);
          hoc_register_npy_direct(mech_type, npy_direct_func_proc);
      }
  }" contains: "struct pas_test_Instance  {
          double* g{};
          double* e{};
          double* i{};
          double* ar{};
          double* s{};
          double* ena{};
          double* ina{};
          double* Ds{};
          double* v_unused{};
          double* g_unused{};
          const double* const* ion_ena{};
          double* const* ion_ina{};
          double* const* ion_dinadv{};
          pas_test_Store* global{&pas_test_global};
      };"

-------------------------------------------------------------------------------
Scenario: Check NEURON codegen for simple MOD file
      Given: A simple mod file with RANGE, ARRAY and ION variables
       Then: Placeholder nrn_cur function is printed
-------------------------------------------------------------------------------
/home/lucg/git/bbp/nmodl/test/unit/codegen/codegen_neuron_cpp_visitor.cpp:223
...............................................................................

/home/lucg/git/bbp/nmodl/test/unit/codegen/codegen_neuron_cpp_visitor.cpp:227: FAILED:
  REQUIRE_THAT( generated, ContainsSubstring(reindent_and_trim_text(expected_placeholder_nrn_cur)) )
with expansion:
  "/*********************************************************
  Model Name      : pas_test
  Filename        : _test.mod
  NMODL Version   : 7.7.0
  Vectorized      : true
  Threadsafe      : true
  Created         : Thu Feb 22 15:59:48 2024
  Simulator       : NEURON
  Backend         : C++ (api-compatibility)
  NMODL Compiler  : 0.6 [eb3215b7 2024-02-22 15:59:27 +0100]
  *********************************************************/
  
#include <math.h>
  #include <stdio.h>
  #include <stdlib.h>
  
#include "mech_api.h"
  #include "neuron/cache/mechanism_range.hpp"
  #include "nrniv_mf.h"
  #include "section_fwd.hpp"
  
/* NEURON global macro definitions */
  /* VECTORIZED */
  #define NRN_VECTORIZED 1
  
static constexpr auto number_of_datum_variables = 3;
  static constexpr auto number_of_floating_point_variables = 10;
  
namespace {
  template <typename T>
  using _nrn_mechanism_std_vector = std::vector<T>;
  using _nrn_model_sorted_token = neuron::model_sorted_token;
  using _nrn_mechanism_cache_range = neuron::cache::MechanismRange
  <number_of_floating_point_variables, number_of_datum_variables>;
  using _nrn_mechanism_cache_instance = neuron::cache::MechanismInstance
  <number_of_floating_point_variables, number_of_datum_variables>;
  using _nrn_non_owning_id_without_container = neuron::container::
  non_owning_identifier_without_container;
  template <typename T>
  using _nrn_mechanism_field = neuron::mechanism::field<T>;
  template <typename... Args>
  void _nrn_mechanism_register_data_fields(Args&&... args) {
      neuron::mechanism::register_data_fields(std::forward<Args>(args)...);
  }
  }  // namespace
  
Prop* hoc_getdata_range(int type);
  

  namespace neuron {
      #ifndef NRN_PRCELLSTATE
      #define NRN_PRCELLSTATE 0
      #endif
  

      /** channel information */
      static const char *mechanism_info[] = {
          "7.7.0",
          "pas_test",
          "g_pas_test",
          "e_pas_test",
          0,
          "i_pas_test",
          "ar_pas_test[2]",
          0,
          "s_pas_test",
          0,
          0
      };
  

      /* NEURON global variables */
      static neuron::container::field_index _slist1[1], _dlist1[1];
      static Symbol* _na_sym;
      static int mech_type;
      static Prop* _extcall_prop;
      /* _prop_id kind of shadows _extcall_prop to allow validity checking. */
      static _nrn_non_owning_id_without_container _prop_id{};
      static int hoc_nrnpointerindex = -1;
      static _nrn_mechanism_std_vector<Datum> _extcall_thread;
  

      /** all global variables */
      struct pas_test_Store {
          double s0{};
      };
      static_assert(std::is_trivially_copy_constructible_v<pas_test_Store>);
      static_assert(std::is_trivially_move_constructible_v<pas_test_Store>);
      static_assert(std::is_trivially_copy_assignable_v<pas_test_Store>);
      static_assert(std::is_trivially_move_assignable_v<pas_test_Store>);
      static_assert(std::is_trivially_destructible_v<pas_test_Store>);
      pas_test_Store pas_test_global;
  

      /** all mechanism instance variables and global variables */
      struct pas_test_Instance  {
          int const * _nodeindices;
          double const * _node_voltages;
          double * _node_rhs;
          double* g{};
          double* e{};
          double* i{};
          double* ar{};
          double* s{};
          double* ena{};
          double* ina{};
          double* Ds{};
          double* v_unused{};
          double* g_unused{};
          const double* const* ion_ena{};
          double* const* ion_ina{};
          double* const* ion_dinadv{};
          pas_test_Store* global{&pas_test_global};
      };
  

      static pas_test_Instance make_instance_pas_test
  (_nrn_mechanism_cache_range& _ml, NrnThread& _nt, Memb_list& _ml_arg) {
          return pas_test_Instance {
              _ml_arg.nodeindices,
              _nt.node_voltage_storage(),
              _nt.node_rhs_storage(),
              _ml.template fpfield_ptr<0>(),
              _ml.template fpfield_ptr<1>(),
              _ml.template fpfield_ptr<2>(),
              _ml.template data_array_ptr<3, 2>(),
              _ml.template fpfield_ptr<4>(),
              _ml.template fpfield_ptr<5>(),
              _ml.template fpfield_ptr<6>(),
              _ml.template fpfield_ptr<7>(),
              _ml.template fpfield_ptr<8>(),
              _ml.template fpfield_ptr<9>(),
              _ml.template dptr_field_ptr<0>(),
              _ml.template dptr_field_ptr<1>(),
              _ml.template dptr_field_ptr<2>()
          };
      }
  

      static void nrn_alloc_pas_test(Prop* _prop) {
          Prop *prop_ion{};
          Datum *_ppvar{};
          _ppvar = nrn_prop_datum_alloc(mech_type, 3, _prop);
          _nrn_mechanism_access_dparam(_prop) = _ppvar;
          _nrn_mechanism_cache_instance _ml_real{_prop};
          auto* const _ml = &_ml_real;
          size_t const _iml{};
          assert(_nrn_mechanism_get_num_vars(_prop) == 10);
          /*initialize range parameters*/
          _ml->template fpfield<0>(_iml) = 0.001; /* g */
          _ml->template fpfield<1>(_iml) = -70; /* e */
          _nrn_mechanism_access_dparam(_prop) = _ppvar;
          Symbol * na_sym = hoc_lookup("na_ion");
          Prop * na_prop = need_memb(na_sym);
          _ppvar[0] = _nrn_mechanism_get_param_handle(na_prop, 0);
          _ppvar[1] = _nrn_mechanism_get_param_handle(na_prop, 3);
      }
  

      /* Neuron setdata functions */
      extern void _nrn_setdata_reg(int, void(*)(Prop*));
      static void _setdata(Prop* _prop) {
          _extcall_prop = _prop;
          _prop_id = _nrn_get_prop_id(_prop);
      }
      static void _hoc_setdata() {
          Prop *_prop = hoc_getdata_range(mech_type);
          _setdata(_prop);
          hoc_retpushx(1.);
      }
      /* Mechanism procedures and functions */
  

      /** connect global (scalar) variables to hoc -- */
      static DoubScal hoc_scalar_double[] = {
          {nullptr, nullptr}
      };
  

      /** connect global (array) variables to hoc -- */
      static DoubVec hoc_vector_double[] = {
          {nullptr, nullptr, 0}
      };
  

      /* declaration of user functions */
  

      /* connect user functions to hoc names */
      static VoidFunc hoc_intfunc[] = {
          {"setdata_pas_test", _hoc_setdata},
          {0, 0}
      };
      static NPyDirectMechFunc npy_direct_func_proc[] = {
      };
  

      void nrn_init_pas_test(_nrn_model_sorted_token const& _sorted_token,
  NrnThread* _nt, Memb_list* _ml_arg, int _type) {
          _nrn_mechanism_cache_range _lmr{_sorted_token, *_nt, *_ml_arg, _type}
  ;
          auto inst = make_instance_pas_test(_lmr, *_nt, *_ml_arg);
          auto nodecount = _ml_arg->nodecount;
          for (int id = 0; id < nodecount; id++) {
              inst.ena[id] = (*inst.ion_ena[id]);
              (inst.ar+id*2)[static_cast<int>(0)] = 1.0;
          }
      }
  

      inline double nrn_current_pas_test(size_t id, pas_test_Instance& inst,
  double v) {
          double current = 0.0;
          inst.i[id] = inst.g[id] * (v - inst.e[id]);
          inst.ina[id] = inst.g[id] * (v - inst.ena[id]);
          current += inst.i[id];
          current += inst.ina[id];
          return current;
      }
  

      /** update current */
      void nrn_cur_pas_test(_nrn_model_sorted_token const& _sorted_token,
  NrnThread* _nt, Memb_list* _ml_arg, int _type) {
          _nrn_mechanism_cache_range _lmr{_sorted_token, *_nt, *_ml_arg, _type}
  ;
          auto inst = make_instance_pas_test(_lmr, *_nt, *_ml_arg);
          auto nodecount = _ml_arg->nodecount;
          for (int id = 0; id < nodecount; id++) {
              int node_id = inst._nodeindices[id];
              double v = inst._node_voltages[node_id];
              inst.ena[id] = (*inst.ion_ena[id]);
              double I1 = nrn_current_pas_test(id, inst, v+0.001);
              double dina = inst.ina[id];
              double I0 = nrn_current_pas_test(id, inst, v);
              double rhs = I0;
              double g = (I1-I0)/0.001;
              (*inst.ion_dinadv[id]) += (dina-inst.ina[id])/0.001;
              (*inst.ion_ina[id]) += inst.ina[id];
              inst._node_rhs[node_id] -= rhs;
          }
      }
  

      void nrn_state_pas_test(_nrn_model_sorted_token const& _sorted_token,
  NrnThread* _nt, Memb_list* _ml_arg, int _type) {
          _nrn_mechanism_cache_range _lmr{_sorted_token, *_nt, *_ml_arg, _type}
  ;
          auto inst = make_instance_pas_test(_lmr, *_nt, *_ml_arg);
          auto nodecount = _ml_arg->nodecount;
          for (int id = 0; id < nodecount; id++) {
              inst.ena[id] = (*inst.ion_ena[id]);
              inst.s[id] = inst.s[id] - _nt->_dt * ( -((inst.ar+id*2)
  [static_cast<int>(0)]));
          }
      }
  

      /** nrn_jacob function */
      static void nrn_jacob_pas_test(_nrn_model_sorted_token const&
  _sorted_token, NrnThread* _nt, Memb_list* _ml_arg, int _type) {}
  

      static void _initlists() {
          /* s */
          _slist1[0] = {4, 0};
          /* Ds */
          _dlist1[0] = {7, 0};
      }
  

      /** register channel with the simulator */
      extern "C" void __test_reg() {
          _initlists();
  
        ion_reg("na", -10000.);
  
        _na_sym = hoc_lookup("na_ion");
  
        register_mech(mechanism_info, nrn_alloc_pas_test, nrn_cur_pas_test,
  nrn_jacob_pas_test, nrn_state_pas_test, nrn_init_pas_test,
  hoc_nrnpointerindex, 1);
  
        mech_type = nrn_get_mechtype(mechanism_info[1]);
          _nrn_mechanism_register_data_fields(mech_type,
              _nrn_mechanism_field<double>{"g"} /* 0 */,
              _nrn_mechanism_field<double>{"e"} /* 1 */,
              _nrn_mechanism_field<double>{"i"} /* 2 */,
              _nrn_mechanism_field<double>{"ar", 2} /* 3 */,
              _nrn_mechanism_field<double>{"s"} /* 4 */,
              _nrn_mechanism_field<double>{"ena"} /* 5 */,
              _nrn_mechanism_field<double>{"ina"} /* 6 */,
              _nrn_mechanism_field<double>{"Ds"} /* 7 */,
              _nrn_mechanism_field<double>{"v_unused"} /* 8 */,
              _nrn_mechanism_field<double>{"g_unused"} /* 9 */,
              _nrn_mechanism_field<double*>{"ion_ena", "na_ion"} /* 0 */,
              _nrn_mechanism_field<double*>{"ion_ina", "na_ion"} /* 1 */,
              _nrn_mechanism_field<double*>{"ion_dinadv", "na_ion"} /* 2 */
          );
  
        hoc_register_prop_size(mech_type, 10, 3);
          hoc_register_dparam_semantics(mech_type, 0, "na_ion");
          hoc_register_dparam_semantics(mech_type, 1, "na_ion");
          hoc_register_dparam_semantics(mech_type, 2, "na_ion");
          hoc_register_var(hoc_scalar_double, hoc_vector_double, hoc_intfunc);
          hoc_register_npy_direct(mech_type, npy_direct_func_proc);
      }
  }" contains: "void nrn_cur_pas_test(_nrn_model_sorted_token const&
  _sorted_token, NrnThread* _nt, Memb_list* _ml_arg, int _type) {}"

===============================================================================
test cases:  1 |  0 passed | 1 failed
assertions: 14 | 12 passed | 2 failed

1uc · 2024-02-22T15:11:14Z

The diff based approach isn't stellar, but at least I can:

see which lines changed,
open the original reference file,
easily obtain a file containing the generated code with the current version of nmodl.

I would also say that one can quickly identify that like these changes (if they were correct and would compile) would likely not affect any of the computations. They're just slightly rearranging some parameters passed to certain NMODL generated functions.

1uc · 2024-02-22T15:16:13Z

I think the unit-test approach isn't sensitive to this (precise) change (which is interesting):

>     inline int set_x_42_test_func_proc_pnt(size_t id, test_func_proc_pnt_Instance& inst, double v) {
>         auto inst = make_instance_test_func_proc_pnt(*_ml, *_nt, *_ml_arg);
251,252c257,258
<     inline int set_x_a_test_func_proc_pnt(_nrn_mechanism_cache_range* _ml, size_t id, Datum* _ppvar, Datum* _thread, NrnThread* _nt, double a) {
<         auto inst = make_instance_test_func_proc_pnt(*_ml);

1uc · 2024-02-22T15:31:54Z

The ugly format can easily be fixed. Yielding the following output:

diff -U 8 -r /home/lucg/git/bbp/nmodl/test/usecases/references/func_proc_pnt/neuron/func_proc_pnt.cpp /tmp/tmp.uWHctFHaFr/neuron/func_proc_pnt.cpp
--- /home/lucg/git/bbp/nmodl/test/usecases/references/func_proc_pnt/neuron/func_proc_pnt.cpp	2024-02-22 15:58:05.526563379 +0100
+++ /tmp/tmp.uWHctFHaFr/neuron/func_proc_pnt.cpp	2024-02-22 16:29:47.645989010 +0100
@@ -77,25 +77,31 @@
     static_assert(std::is_trivially_copy_assignable_v<test_func_proc_pnt_Store>);
     static_assert(std::is_trivially_move_assignable_v<test_func_proc_pnt_Store>);
     static_assert(std::is_trivially_destructible_v<test_func_proc_pnt_Store>);
     test_func_proc_pnt_Store test_func_proc_pnt_global;
 
 
     /** all mechanism instance variables and global variables */
     struct test_func_proc_pnt_Instance  {
+        int const * _nodeindices;
+        double const * _node_voltages;
+        double * _node_rhs;
         double* x{};
         double* v_unused{};
         const double* const* node_area{};
         test_func_proc_pnt_Store* global{&test_func_proc_pnt_global};
     };
 
 
-    static test_func_proc_pnt_Instance make_instance_test_func_proc_pnt(_nrn_mechanism_cache_range& _ml) {
+    static test_func_proc_pnt_Instance make_instance_test_func_proc_pnt(_nrn_mechanism_cache_range& _ml, NrnThread& _nt, Memb_list& _ml_arg) {
         return test_func_proc_pnt_Instance {
+            _ml_arg.nodeindices,
+            _nt.node_voltage_storage(),
+            _nt.node_rhs_storage(),
             _ml.template fpfield_ptr<0>(),
             _ml.template fpfield_ptr<1>(),
             _ml.template dptr_field_ptr<0>()
         };
     }
 
 
     static void nrn_alloc_test_func_proc_pnt(Prop* _prop) {
@@ -138,19 +144,19 @@
...

bbpbuildbot · 2024-02-22T16:13:25Z

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

Status and direct links:

1uc · 2024-02-23T10:21:42Z

We can now precisely point out issues, e.g. here:
#1168

at time of writing my copy of NMODL generates faulty code that I can't copy-paste, but since these references are stored I can copy/link from there.

.gitmodules

test/usecases/generate_references.sh

test/usecases/CMakeLists.txt

JCGoran

LGTM

Add a script for generating the reference values of compiled mod files. These are included with the sources to enable the following: * during review the reviewer can see what was generated. * these can serve as golden tests, flagging changes in unrelated mod files.

Co-authored-by: JCGoran <[email protected]>

Add a script for generating the reference values of compiled mod files. These are included with the sources to enable the following: * during review the reviewer can see what was generated. * these can serve as golden tests, flagging changes in unrelated mod files. --------- Co-authored-by: JCGoran <[email protected]>

1uc commented Dec 20, 2023

View reviewed changes

test/usecases/cnexp_scalar/leonhard.cpp Outdated Show resolved Hide resolved

1uc commented Dec 20, 2023

View reviewed changes

test/usecases/cnexp_scalar/leonhard.cpp Outdated Show resolved Hide resolved

1uc force-pushed the 1uc/sketch-golden-tests branch from 2492b8c to 16d82cd Compare December 20, 2023 09:09

iomaganaris added the NEURON codegen Work toward NEURON code generation label Dec 20, 2023

1uc marked this pull request as ready for review December 20, 2023 09:35