Skip to content

Modernise underlying data structure for node voltages#1929

Closed
olupton wants to merge 209 commits into
masterfrom
olupton/data-structures
Closed

Modernise underlying data structure for node voltages#1929
olupton wants to merge 209 commits into
masterfrom
olupton/data-structures

Conversation

@olupton

@olupton olupton commented Jul 21, 2022

Copy link
Copy Markdown
Collaborator

First attempt at importing the ideas of https://github.com/neuronsimulator/data-structure-design into NEURON.

Very WIP, detailed review not advised.

@olupton olupton force-pushed the olupton/data-structures branch from 22ac03b to bd73ecb Compare July 25, 2022 10:37
olupton
olupton commented Jul 26, 2022
View reviewed changes
Comment thread .github/workflows/neuron-ci.yml Outdated
olupton
olupton commented Jul 26, 2022
View reviewed changes
Comment thread CMakeLists.txt Outdated
@olupton olupton mentioned this pull request Jul 26, 2022
Open
@olupton olupton force-pushed the olupton/data-structures branch from 0606a9f to 52c54e0 Compare August 30, 2022 15:56
@olupton olupton mentioned this pull request Aug 30, 2022
Merged
@nrnhines

nrnhines commented Aug 31, 2022

Copy link
Copy Markdown
Member

I'm delighted to see that nrn/test/datahandle/test.py is working without address sanitizer complaints!

Compiling did require:

$ git diff src/oc/list.cpp
diff --git a/src/oc/list.cpp b/src/oc/list.cpp
index eecbd2d3..c31dd281 100644
--- a/src/oc/list.cpp
+++ b/src/oc/list.cpp
@@ -28,13 +28,13 @@ It is intended that this implementation be hidden from the user via the
 following function calls.
 */
 
-#define HOC_L_LIST 1
-
 #include <stdlib.h>
-#include "hoclist.h"
 #include "hocdec.h"
 #include "parse.hpp"
 
+#define HOC_L_LIST 1
+#include "hoclist.h"
+
 #define Free free
 
 static Item* newitem(void) {

to avoid issues with

#define next        hoc_l_next

but I think you already resolved that in another PR. If not, let me know and I'll push this change to this PR.

@olupton olupton force-pushed the olupton/data-structures branch 2 times, most recently from 6e101bd to 0f9f7fc Compare September 1, 2022 09:25
@alexsavulescu alexsavulescu force-pushed the olupton/data-structures branch from 321815f to bf5e48c Compare September 4, 2022 08:58
@olupton olupton mentioned this pull request Sep 5, 2022
Merged
@olupton olupton force-pushed the olupton/data-structures branch 5 times, most recently from dd0dae2 to 47194d1 Compare September 9, 2022 12:06
@olupton olupton mentioned this pull request Sep 14, 2022
Closed
@codecov-commenter

codecov-commenter commented Sep 14, 2022 •
edited by codecov Bot
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.55929% with 199 lines in your changes missing coverage. Please review.
✅ Project coverage is 47.85%. Comparing base (39e36db) to head (1ba1383).
⚠️ Report is 2869 commits behind head on master.

Files with missing lines Patch % Lines
src/nrniv/kschan.cpp 22.41% 45 Missing ⚠️
src/nrnoc/cabcode.cpp 82.17% 18 Missing ⚠️
src/nrnoc/secref.cpp 0.00% 16 Missing ⚠️
src/nrnpython/nrnpy_hoc.cpp 75.80% 15 Missing ⚠️
src/nrnpython/nrnpy_nrn.cpp 75.67% 9 Missing ⚠️
src/nrniv/kssingle.cpp 0.00% 7 Missing ⚠️
src/nrniv/nonlinz.cpp 68.18% 7 Missing ⚠️
src/neuron/container/soa_identifier.hpp 89.47% 6 Missing ⚠️
src/nrncvode/netcvode.cpp 95.97% 6 Missing ⚠️
src/nrniv/bbsavestate.cpp 87.75% 6 Missing ⚠️
... and 22 more
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #1929      +/-   ##
==========================================
+ Coverage   47.51%   47.85%   +0.34%     
==========================================
  Files         528      541      +13     
  Lines      118852   119572     +720     
==========================================
+ Hits        56468    57223     +755     
+ Misses      62384    62349      -35

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@alexsavulescu alexsavulescu force-pushed the olupton/data-structures branch 12 times, most recently from 94dc46f to 8bc8860 Compare September 18, 2022 13:27
olupton added 16 commits October 10, 2022 10:31
@olupton olupton force-pushed the olupton/data-structures branch from 9069acc to e30394b Compare October 10, 2022 08:35
olupton added a commit that referenced this pull request Oct 10, 2022
@olupton
Pick up minor improvements from #1929 (#2015)
67f9bd2 
- STETransition et al. now use STL components.
- Removal of #if 0 block in nrn_finitialize fixes formatting there.
- table_check_ no longer uses Datum.
- PyHocObject is consistently defined in only one place.
- Exception messages are printed to stderr when they are caught.
- example_nmodl tests are unpacked into different tests, Python
  versions are run (not just HOC).
- "Test fast_imem calculation" tests now check status codes.
@alexsavulescu alexsavulescu mentioned this pull request Oct 12, 2022
Merged
@olupton olupton mentioned this pull request Oct 17, 2022
Merged
6 tasks
alexsavulescu added a commit that referenced this pull request Oct 21, 2022
@alexsavulescu
add a wrapper around Datum
b51ec82 
* hoist and match several changes from #1929 in order to reduce its diff
* hack _pval <-> pval for backward MDB compatibility
@olupton

olupton commented Nov 3, 2022

Copy link
Copy Markdown
Collaborator Author

Closing this as it is included in #2027.

@olupton olupton closed this Nov 3, 2022
olupton pushed a commit that referenced this pull request Nov 3, 2022
@alexsavulescu @olupton
add a wrapper around Datum
f827aa3 
* hoist and match several changes from #1929 in order to reduce its diff
* hack _pval <-> pval for backward MDB compatibility
alexsavulescu added a commit that referenced this pull request Nov 7, 2022
@alexsavulescu @olupton
add a wrapper around Datum (#2022)
44480b7 
* add a wrapper around `Datum`
  * hoist and match several changes from #1929 in order to reduce its diff
  * hack _pval <-> pval for backward MDB compatibility
  * implement .get<T>

Co-authored-by: Olli Lupton <oliver.lupton@epfl.ch>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexsavulescu alexsavulescu alexsavulescu left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@olupton @nrnhines @codecov-commenter @alexsavulescu