Skip to content

cvode no_cap uses direct backing store instead of Node*#3314

Merged
nrnhines merged 38 commits into
masterfrom
hines/cellcontig-treemat2
Jun 4, 2025
Merged

cvode no_cap uses direct backing store instead of Node*#3314
nrnhines merged 38 commits into
masterfrom
hines/cellcontig-treemat2

Conversation

@nrnhines

@nrnhines nrnhines commented Jan 21, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

This is the latest in a series of PR's and branches that are meant to improve cvode performance to the level of release/8.2
The series is
hines/cvlocal-bug #3299
hines/cell-contiguous #3300
hines/cellcontig+scatter-ptr https://github.com/neuronsimulator/nrn/tree/hines/cellcontig%2Bscatter-ptr
hines/cellcontig-scatter-treemat https://github.com/neuronsimulator/nrn/tree/hines/cellcontig-scatter-treemat
this PR

This PR generally approaches or exceeds the performance of 8.2 (at least for x86_64) with respect to cvode. Ringtest performance is (data copied from #3283 (comment))

nrniv -python ringtest.py -npt 1 -compart 1 3 -nring 32 -rparm -nt 1

x86_64 intel icx
version  fixed  global  local
8.2      1.86   30.5    1.35
master   1.53   56.0    6.67
contig   1.59   60.8    2.76
scatter  1.71   48.4    2.32
treemat  1.61   35.9    2.01
nocap    1.6    25.1    1.78

x86_64 gcc 13.3.0
version  fixed  global  local
8.2          2.92   51.9    2.47
master   2.38   63.4    6.19
contig    2.09   63.6    3.10
scatter   1.98   49.8    2.54
treemat  2.01   38.7    2.25
nocap     2.01   28.4    2.01

There continues to be an issue with Apple M1

Apple M1
version  fixed  global  local
8.2         1.87    23.67   1.32
master  4.1      60.1      5.3
contig    5.0      70.5      4.1
scatter   4.9      64.6      3.8
treemat 4.1      47.5      2.9
nocap    4.9      58.3      3.47
coreneuron 2.0

@nrnhines
cvode scatter/gather use double* instead of data_handle
ddbee41
@nrnhines
clang format
9f9e5fb
@nrnhines
Lvardt minimize size of CvMembList.ml Memb_list vector.
d7778e2
@nrnhines
For lvardt, allow CvMembList.ml.size() > 1 and nodecount > 1
5c1537b
@nrnhines
fix lvardt segfault when a cvode instance is empty.
01b4149
@nrnhines
clang format
d73896e
@nrnhines
Default node order is cell contiguous (except for thread rootnodes).
f2c2f28
@nrnhines
Merge remote-tracking branch 'origin/hines/cvscatter-ptr' into hines/…
18a4aea 
…cellcontig+scatter-ptr
@nrnhines
update to ringtest that supports -method and -2nd_order_thresh
c8988f1
@nrnhines
incorporate reviewer suggestions
11ba674
@nrnhines
Full coverage for this PR.
ac1c6a9
@nrnhines
Merge branch 'hines/cvlocal-bug' into hines/cell-contiguous
4f6cba4
@nrnhines
Merge branch 'hines/cell-contiguous' into hines/cellcontig+scatter-ptr
57a987e
@nrnhines
Merge branch 'master' into hines/cvlocal-bug
8e1e0e5
@nrnhines
Merge branch 'hines/cvlocal-bug' into hines/cell-contiguous
7a505e0
@nrnhines
Merge branch 'hines/cell-contiguous' into hines/cellcontig+scatter-ptr
d767760
@nrnhines
For cvode matrix setup/solve use soa storage directly instead of Node**.
53010db
@nrnhines
fix bug in contiguity constraint
f7a2872
@nrnhines
cvode no_cap uses direct backing store instead of Node*
64548b9
@nrnhines
Node** v_node_, etc., removed from CvodeThreadData.
89baa0d
@nrnhines
clang format
35c2687
@nrnhines
Determine correct thread for Cvode with CvodeThreadData.
2395595
@nrnhines
Merge branch 'master' into hines/cellcontig-treemat2
bbedc36
@nrnhines
Some extra coverage
96b78c6
@nrnhines
Merge branch 'master' into hines/cellcontig-treemat2
2123efe
@sonarqubecloud

sonarqubecloud Bot commented Jan 21, 2025

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
87 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@azure-pipelines

azure-pipelines Bot commented Jan 21, 2025

Copy link
Copy Markdown

✔️ 2123efe -> Azure artifacts URL

@codecov

codecov Bot commented Jan 21, 2025 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.42%. Comparing base (0b9477a) to head (051f346).
⚠️ Report is 61 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3314      +/-   ##
==========================================
+ Coverage   68.38%   68.42%   +0.04%     
==========================================
  Files         682      682              
  Lines      116556   116565       +9     
==========================================
+ Hits        79705    79760      +55     
+ Misses      36851    36805      -46

☔ 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.

@github-actions

github-actions Bot commented May 26, 2025

Copy link
Copy Markdown
Contributor

✔️ 865a22c -> artifacts URL

@azure-pipelines

azure-pipelines Bot commented May 31, 2025

Copy link
Copy Markdown

✔️ 2e126df -> Azure artifacts URL

@github-actions

github-actions Bot commented May 31, 2025

Copy link
Copy Markdown
Contributor

✔️ 2e126df -> artifacts URL

@github-actions

github-actions Bot commented Jun 2, 2025

Copy link
Copy Markdown
Contributor

✔️ 190c009 -> artifacts URL

@azure-pipelines

azure-pipelines Bot commented Jun 2, 2025

Copy link
Copy Markdown

✔️ 190c009 -> Azure artifacts URL

@github-actions

github-actions Bot commented Jun 2, 2025

Copy link
Copy Markdown
Contributor

✔️ 7caee3c -> artifacts URL

@azure-pipelines

azure-pipelines Bot commented Jun 2, 2025

Copy link
Copy Markdown

✔️ 7caee3c -> Azure artifacts URL

ramcdougal
ramcdougal reviewed Jun 3, 2025
View reviewed changes
Comment thread src/nrncvode/cvodeobj.cpp
ramcdougal
ramcdougal reviewed Jun 3, 2025
View reviewed changes
Comment thread src/nrncvode/cvtrset.cpp Outdated
ramcdougal
ramcdougal reviewed Jun 3, 2025
View reviewed changes
Comment thread src/nrncvode/cvtrset.cpp Outdated
ramcdougal
ramcdougal reviewed Jun 3, 2025
View reviewed changes
Comment thread src/nrncvode/occvode.cpp Outdated
ramcdougal
ramcdougal reviewed Jun 3, 2025
View reviewed changes
Comment thread src/nrncvode/occvode.cpp Outdated
ramcdougal
ramcdougal reviewed Jun 3, 2025
View reviewed changes
Comment thread src/nrncvode/occvode.cpp Outdated
ramcdougal
ramcdougal reviewed Jun 3, 2025
View reviewed changes
Comment thread src/nrncvode/occvode.cpp
@github-actions

github-actions Bot commented Jun 3, 2025

Copy link
Copy Markdown
Contributor

✔️ 12aa723 -> artifacts URL

@azure-pipelines

azure-pipelines Bot commented Jun 3, 2025

Copy link
Copy Markdown

✔️ 12aa723 -> Azure artifacts URL

ramcdougal
ramcdougal reviewed Jun 3, 2025
View reviewed changes
Comment thread src/nrncvode/occvode.cpp Outdated
ramcdougal
ramcdougal reviewed Jun 3, 2025
View reviewed changes
Comment thread src/nrncvode/occvode.cpp Outdated
@sonarqubecloud

sonarqubecloud Bot commented Jun 3, 2025

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
49 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@azure-pipelines

azure-pipelines Bot commented Jun 3, 2025

Copy link
Copy Markdown

✔️ 051f346 -> Azure artifacts URL

@github-actions

github-actions Bot commented Jun 3, 2025

Copy link
Copy Markdown
Contributor

✔️ 74141e7 -> artifacts URL

@github-actions

github-actions Bot commented Jun 3, 2025

Copy link
Copy Markdown
Contributor

✔️ 051f346 -> artifacts URL

@nrnhines nrnhines merged commit b37e37c into master Jun 4, 2025
43 checks passed
@nrnhines nrnhines deleted the hines/cellcontig-treemat2 branch June 4, 2025 00:27
@nrnhines nrnhines mentioned this pull request Jun 8, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ramcdougal ramcdougal ramcdougal approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nrnhines @ramcdougal