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

Integrate the entire dynamics for higher-order ODEs #1139

Merged
merged 14 commits into from
Nov 27, 2024

Conversation

pnbabu
Copy link
Contributor

@pnbabu pnbabu commented Nov 6, 2024

No description provided.

@pnbabu pnbabu requested a review from clinssen November 6, 2024 18:23
Copy link

github-actions bot commented Nov 7, 2024

🐰 Bencher Report

Branch1139/merge
Testbedubuntu-latest

⚠️ WARNING: No Threshold found!

Without a Threshold, no Alerts will ever be generated.

Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the --ci-only-thresholds CLI flag.

Click to view all benchmark results
BenchmarkLatencynanoseconds (ns)
tests/nest_continuous_benchmarking/test_nest_continuous_benchmarking.py::TestNESTContinuousBenchmarking::test_stdp_nn_synapse📈 view plot
⚠️ NO THRESHOLD
3,954,045,038.80
🐰 View full continuous benchmarking report in Bencher

fig.savefig("/tmp/test_integrate_odes_higher_order.png", dpi=300)

# verify
np.testing.assert_allclose(x[-1], 0.10737970490959549)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we know these values are correct (are they from just running the NESTML model once, or from an independent implementation of the ODEs)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These values are from repeated simulations of the NESTML-generated code, equivalent to providing integrate_odes(x',x) in the NESTML model. I have changed the test now to include calls to both integrate_odes and compare their output.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the latest changes, integrate_odes(x, x') is not supported and hence I changed the test again to compare it against numerical values. Do you have any other ideas to check this?

@clinssen
Copy link
Contributor

Thanks for the PR! Could you add/update the documentation for the integrate_odes() function as well?

doc/nestml_language/nestml_language_concepts.rst Outdated Show resolved Hide resolved
doc/nestml_language/nestml_language_concepts.rst Outdated Show resolved Hide resolved
integrate_odes(x)
# integrate_odes(x', x) # equivalent to integrate_odes(x)

Here, ``integrate_odes(x)`` integrates the entire dynamics of ``x(t)`` and is equivalent to ``integrate_odes(x', x)``.
Copy link
Contributor

@clinssen clinssen Nov 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be allowed to write variables with derivatives inside integrate_odes() at all? As in, x is allowed, but x' is not. Otherwise, what is the semantics of saving integrate_odes(x')?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have now updated one of the existing cocos to check this condition.

Copy link
Contributor

@clinssen clinssen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you!

@pnbabu pnbabu merged commit 806ca13 into nest:master Nov 27, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants