Skip to content

Improve the time taken to validate and analyse large models#1397

Merged
agarny merged 46 commits into
cellml:mainfrom
hsorby:issue1396
Jun 25, 2026
Merged

Improve the time taken to validate and analyse large models#1397
agarny merged 46 commits into
cellml:mainfrom
hsorby:issue1396

Conversation

@hsorby

@hsorby hsorby commented May 19, 2026

Copy link
Copy Markdown
Contributor

To run the investigation tests set an environment variable BENCHMARKING_MODEL_ROOT to the root directory of the unzipped contents of the benchmarking files zip in the associated issue.

The longer tests are disabled with the DISABLED_ prefix on the test name, remove this prefix to run the test.

@hsorby hsorby marked this pull request as ready for review June 20, 2026 11:04
@hsorby hsorby requested a review from agarny June 20, 2026 11:04
agarny
agarny previously approved these changes Jun 22, 2026

@agarny agarny 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 good although I have sped up a few things here and there (see commit messages).

@agarny

agarny commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

To run the investigation tests set an environment variable BENCHMARKING_MODEL_ROOT to the root directory of the unzipped contents of the benchmarking files zip in the associated issue.

The longer tests are disabled with the DISABLED_ prefix on the test name, remove this prefix to run the test.

I can't see where BENCHMARKING_MODEL_ROOT and the DISABLED_ prefix are being used in this PR...?

@hsorby

hsorby commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

To run the investigation tests set an environment variable BENCHMARKING_MODEL_ROOT to the root directory of the unzipped contents of the benchmarking files zip in the associated issue.
The longer tests are disabled with the DISABLED_ prefix on the test name, remove this prefix to run the test.

I can't see where BENCHMARKING_MODEL_ROOT and the DISABLED_ prefix are being used in this PR...?

I have taken that bit out now, as this was only part of the investigation phase of this work.
It was there for people to be able to run the models given in the associated issue easily.

@agarny agarny 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 good to me.

@hsorby hsorby requested review from agarny and jmdowrick June 22, 2026 23:15
agarny
agarny previously approved these changes Jun 22, 2026
Comment thread src/analysermodel.cpp
Comment thread src/validator.cpp Outdated
jmdowrick
jmdowrick previously approved these changes Jun 24, 2026

@jmdowrick jmdowrick left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks OK to me. Some nice changes.

@hsorby hsorby dismissed stale reviews from jmdowrick and agarny via be20705 June 25, 2026 03:15
@agarny agarny merged commit b065011 into cellml:main Jun 25, 2026
14 checks passed
@hsorby hsorby deleted the issue1396 branch June 25, 2026 03:26
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.

3 participants