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

Clean up NESTML grammar definition #1144

Merged
merged 6 commits into from
Dec 2, 2024
Merged

Conversation

clinssen
Copy link
Contributor

@clinssen clinssen commented Nov 28, 2024

One of the unit tests (nestml_printer_test.py) was silently failing on master, giving parse errors which were not resulting in exceptions/test failures.

This PR fixes the issue and cleans up (internally refactors) the NESTML grammar definition.

The term "block" only refers to top-level elements in a model (update, state, parameters, onReceive, etc.) The inner "ASTBlock" was renamed to ASTStmtsBody as it includes an INDENT in the grammar rule.

Copy link

github-actions bot commented Nov 28, 2024

🐰 Bencher Report

Branch1144/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
4,008,302,939.20
🐰 View full continuous benchmarking report in Bencher

@clinssen clinssen marked this pull request as ready for review November 28, 2024 13:32
Copy link
Contributor

@pnbabu pnbabu left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this! I have two minor comments.

pynestml/grammars/PyNestMLParser.g4 Outdated Show resolved Hide resolved
tests/test_nestml_printer.py Outdated Show resolved Hide resolved
tests/test_nestml_printer.py Show resolved Hide resolved
Copy link
Contributor

@pnbabu pnbabu left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@clinssen
Copy link
Contributor Author

clinssen commented Dec 2, 2024

Great, thank you!

@clinssen clinssen merged commit 8e9b564 into nest:master Dec 2, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants