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.

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

NEURON 9.0 release #3137

Open
1 of 29 tasks
pramodk opened this issue Oct 21, 2024 · 9 comments
Open
1 of 29 tasks

NEURON 9.0 release #3137

pramodk opened this issue Oct 21, 2024 · 9 comments
Assignees
Labels
release Release related issues

Comments

@pramodk
Copy link
Member

pramodk commented Oct 21, 2024

Action items

Pre-release

Sanity checks

  • Create release/x.y branch and make sure GitHub, Azure and CircleCI builds pass
  • Run nrn-build-ci for the respective Azure build; see Azure drop guide
  • Activate ReadTheDocs build for release/x.y & make it hidden. Check docs are fine after build is done.
  • Run a test wheel build WITHOUT upload for release/x.y to ensure all the wheels build (see details)
  • Run BBP Simulation Stack & other relevant tests

Releasing

  • Update semantic version in CMakeLists.txt
  • Update changelog below and agree on it with everyone; then commit it to docs/changelog (copy structure as-is)
  • Update docs/index.rst accordingly with the new .pkg and .exe links for PKG installer and Windows Installer
  • Run the ReadTheDocs build again for release-x.y, make sure the build passes and inspect the Changelog page.

Important: we print the following message from nrnivmodl on error. Make sure the URL is right one!

NOTE: If you are encountering MOD file compilation errors only with NEURON version 9.0 or later
refer to the C++ migration guide at https://nrn.readthedocs.io/en/9.0.0/guide/porting_mechanisms_to_cpp.html
  • Create new release+tag on GitHub via release workflow. Note that the GitHub release will be marked as pre-release and will contain the full-src-package and the Windows installer at the end of the release workflow.
  • Build release wheels but WITHOUT upload (see details)
  • Create, test and upload manual artifacts
    • MacOS package installer (manual task, ask Michael)
    • arm64 wheels (manual task, check with Erik, Goran or Pramod)
    • aarch64 wheels (create a release/x.y-aarch64 branch for this, see guide)
  • Publish the x.y.z wheels on Pypi; see wheel publishing instructions
  • Once wheels are published, activate the x.y.z tag on ReadTheDocs
  • Rename the Windows installer in the GitHub release to match the new version and the supported python versions (i.e. nrn-8.2.2.w64-mingw-py-37-38-39-310-311-setup.exe
    )
  • Publish release on GitHub (edit https://github.com/neuronsimulator/nrn/releases/tag/x.y.z and un-tick the pre-release checkbox)

Post-release

  • To mark the start of a new development cycle, tag master as follows:
    • minor version: x.(y+1).dev
    • major version: (x+1).0.dev
  • Deactivate ReadTheDocs build for release/x.y
  • Go to ReadTheDocs advanced settings and set Default version to x.y.z
  • Let people know 🚀
  • Cherrypick changelog and installer links to master
  • Update the changelog for the release on GitHub

Changelog

NEURON X.Y

[x.y.z]

Release Date : DD-MM-YYYY

What's New

  • [List new features/support added]
  • .....

Breaking Changes

  • [List the changes that aren't backward compatible]
  • ...

Deprecations

  • [List the features that are deprecated]
  • ...

Bug Fixes

  • [List the important bug fixes]
  • ...

Improvements / Other Changes

  • [List the improvements made in the new release and any other changes]
  • ...

Upgrade Steps

  • [Describe how to migrate from previous NEURON Version]
  • ...

For the complete list of features and bug fixes, see the list in GitHub Issue #[GH_no.]

ReadTheDocs sneak peek

Commits going into x.y.z

[given a.b.c is the last release:]

Since [a.b.c], with:

git log --pretty=format:"%h : %s" a.b.c..release/x.y

we get:

  • commit 1
  • commit 2
  • ...
@pramodk pramodk added the release Release related issues label Oct 21, 2024
@JCGoran
Copy link
Contributor

JCGoran commented Oct 31, 2024

Currently there are some failures in modelDB for version 9.0, as seen here: https://github.com/neuronsimulator/nrn-modeldb-ci/actions/runs/11586042419/job/32255970874
The culprit appears to be this PR: #3093
I tested with a patched version of release 8.2, and the differences between the two (9.0 nightly and patched 8.2) disappear, so the failures are actually a non-issue.

@JCGoran
Copy link
Contributor

JCGoran commented Nov 1, 2024

Somehow #3143 (see #3143 (comment) for reproducer) broke the arm64 build on Linux: https://app.circleci.com/pipelines/github/neuronsimulator/nrn/9533/workflows/f44568f7-581f-4a2b-b9bf-fdf643ba0c14/jobs/4696
so the release is currently blocked.
UPDATE: same PR seems to break on MacOS on arm as well: https://github.com/neuronsimulator/nrn/actions/runs/11632330511/job/32395225512
The "good" news is, if we revert the commit, it passes: https://github.com/neuronsimulator/nrn/actions/runs/11651457741/job/32441452028

@JCGoran
Copy link
Contributor

JCGoran commented Nov 5, 2024

It also seems that this commit (from #3135): 510b9bc
causes some modelDB models to fail: https://github.com/neuronsimulator/nrn-modeldb-ci/actions/runs/11682619896/job/32530121626
I've verified that the parent commit works: https://github.com/neuronsimulator/nrn-modeldb-ci/actions/runs/11682464986/job/32529644621

The issue in #3137 (comment) appears to be unrelated to this one (tested locally by building a wheel on MacOS in a Docker container that runs Linux under arm64).

@nrnhines
Copy link
Member

nrnhines commented Nov 6, 2024

I've looked at the gout for

runmodels --virtual --workdir=8.2.6-g078a34a9d 126467 113997 256388 149100 144520 3800
runmodels --virtual --workdir=9.0a-390-g68b193d8d 126467 113997 256388 149100 144520 3800

and, yes, they all differ. However looking at the trajectories with diffgout they are more similar than I can ever get them to correspond for versions 2 and 3 of cvode at any tolerance. This has been an issue since 1996. And it has always forced me to do manual checking. See #1328 (comment)

@nrnhines
Copy link
Member

nrnhines commented Nov 7, 2024

I added the NRN_ENABLE_ARCH_INDEP_EXP_POW feature to a temporary branch off release/8.2. It is remarkable. that the mere presence of that code (even without activating the mpfr calculation of pow(x, y)) produces identical gout results in comparison with the master for the above models, 126467 113997 256388 149100 144520 3800.

@nrnhines
Copy link
Member

nrnhines commented Nov 7, 2024

In this particular case, it seems isolated to the change (starting from 8.2) src/nmodl/nocpout.cpp

 \n#define exp hoc_Exp\nextern double hoc_Exp(double);\
+\n#undef pow\
+\n#define pow hoc_pow\
 \n#endif\n\

which is used in the translated hh.c file in the line

_lq10 = pow( 3.0 , ( ( celsius - 6.3 ) / 10.0 ) ) ;

@nrnhines
Copy link
Member

nrnhines commented Nov 7, 2024

So, when src/nmodl/nocpout.cpp is modified so that:

+++ b/src/nmodl/nocpout.cpp
@@ -280,8 +280,8 @@ void parout() {
 \n#if !NRNGPU\
 \n#undef exp\
 \n#define exp hoc_Exp\nextern double hoc_Exp(double);\
-\n#undef pow\
-\n#define pow hoc_pow\
+\n static double __attribute__((noinline)) foopow(double x, double y) { return pow(x, y); }\
+\n#define pow foopow\
 \n#endif\n\
 ");

the result are the same except for 1 as the master. But, I guess, the following is inlined

$ for i in `(cd 8.2.6-pow ; find . -name gout)`; do cmp 8.2.6-pow/$i 8.2.6-g078a34a9d/$i ; done
8.2.6-pow/./126467/NegroniLascano/gout 8.2.6-g078a34a9d/./126467/NegroniLascano/gout differ: byte 37, line 4
8.2.6-pow/./149100/HayEtAl2013/gout 8.2.6-g078a34a9d/./149100/HayEtAl2013/gout differ: byte 34, line 4
8.2.6-pow/./256388/PousinhaMouskaBianchiEtAl2019/gout 8.2.6-g078a34a9d/./256388/PousinhaMouskaBianchiEtAl2019/gout differ: byte 36, line 4
8.2.6-pow/./3800/cardiac1998/gout 8.2.6-g078a34a9d/./3800/cardiac1998/gout differ: byte 37, line 4

$ for i in `(cd 8.2.6-pow ; find . -name gout)`; do cmp 8.2.6-pow/$i 9.0a-390-g68b193d8d/$i ; done
8.2.6-pow/./144520/DiFrancescoNoble1985/gout 9.0a-390-g68b193d8d/./144520/DiFrancescoNoble1985/gout differ: byte 36, line 4

but when the __attribute__((noinline)) is commented out then all files are identical to the original 8.2.6-g078a34a9d

@nrnhines
Copy link
Member

nrnhines commented Nov 7, 2024

Chatgpt says:
Reasons for Different Results:
Floating-Point Optimization: When functions are inlined, compilers may apply different optimizations that affect the precision and intermediate calculations of floating-point operations. This can lead to small differences in results due to reordering or additional intermediate calculations.
Register Use and Rounding: Inlined functions may use registers for intermediate results more efficiently than when the function is called separately. This affects rounding behavior because floating-point registers might have extended precision compared to variables stored in memory.
Compiler Optimization Levels: Depending on your optimization flags (e.g., -O2, -O3), the compiler might perform aggressive optimizations, including more extensive inlining and rearrangement of instructions that affect numerical stability.
What to Consider:
Floating-Point Consistency: If precise consistency in floating-point results is critical, you can use compiler flags like -ffloat-store or adjust optimization flags to control floating-point behavior.
Compiler Options: Experiment with different compiler flags (e.g., -O0, -O2, -O3, -march=native) to understand how they affect inlining and floating-point handling.
Function Attributes: Using attribute((noinline)) ensures that the function call is never inlined, which may help maintain consistency when you need exact behavior for debugging or reproducibility.

@nrnhines
Copy link
Member

nrnhines commented Nov 7, 2024

It has been a bit frustrating since none of -ffloat-store or -fexcess-precision=standard or -fno-inline have changed either the master or 8.2 results. Only the __attribute__((noinline)) in 8.2 has had the effect of making (all but one of) the results the same as the master.

It seems that -O2 is a problem. 8.2 produces same results as master when 8.2 is configured with

cmake .. -G Ninja -DCMAKE_INSTALL_PREFIX=install -DCMAKE_BUILD_TYPE=Debug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Release related issues
Projects
None yet
Development

No branches or pull requests

3 participants