Skip to content

Generator, parser and analyser: lookups instead of whole-model scans - #1496

Open
FinbarArgus wants to merge 5 commits into
cellml:mainfrom
FinbarArgus:perf/generator-parser-scaling
Open

FinbarArgus wants to merge 5 commits into
cellml:mainfrom
FinbarArgus:perf/generator-parser-scaling

Conversation

@FinbarArgus

Copy link
Copy Markdown
Contributor

Addresses #1495.

Replaces four whole-model scans with lookups. Generated code and printed models are unchanged.

  • AnalyserModel::analyserVariable(): a map from equivalence group to analyser variable, built at the end of analysis.
  • Generator: each variable's index among the tracked variables of its list, computed once per list and generation.
  • Parser::loadConnection(): components found in a map of the model's components by name.
  • Analyser: the equations computing each variable, gathered in one pass over the equations.

Seconds (Release, GCC 11), for a whole-cell model (25 states, 567 variables) replicated N times:

N generate C, main this PR parse, main this PR
8 1.67 0.05 0.17 0.14
16 8.21 0.12 0.44 0.28
64 — 0.99 — 1.27

Analysis time is mostly validation, which #1477 addresses; with #1477 and #1479, this PR takes analysis at N = 16 from 1.31 s to 0.79 s.

Verification: all 75 test targets pass (GCC, TWAE=ON); LLVM coverage is 100% (regions, functions, lines, branches); clang-format 18 reports no changes; the generated C and printed models at N = 1 and 8 are byte-identical to main.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Ew9LjxY3znMKSNg8AK2kSn

- AnalyserModel::analyserVariable(): a map from equivalence group to analyser variable, built at the end of analysis.
- Generator: each variable's index among the tracked variables of its list, computed once per list and generation.
- Parser: connections find their components in a map of the model's components by name.
- Analyser: the equations computing each variable, gathered in one pass over the equations.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ew9LjxY3znMKSNg8AK2kSn

@hsorby hsorby left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks pretty good, just a small tidy up of the documentation for the buildAnalyserVariableByGroup method.

Comment thread src/analysermodel.cpp Outdated
Comment thread src/analysermodel_p.h Outdated
hsorby
hsorby previously approved these changes Oct 12, 2026

@hsorby hsorby left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I’m happy with this now.

@agarny

agarny commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

I’m happy with this now.

I am not, so I am going to edit the PR so that it looks a bit more like the rest of the analyser/generator code.

@agarny
agarny requested a review from hsorby October 13, 2026 00:14
@jmdowrick

Copy link
Copy Markdown
Contributor

@hsorby

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.

4 participants