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.

Sign up for GitHub

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

Jump to bottom

Added an Analyser class #631

Merged
merged 248 commits into from
Sep 30, 2020
Merged

Added an Analyser class #631

merged 248 commits into from
Sep 30, 2020

Conversation

agarny
Copy link
Contributor

@agarny agarny commented Jun 16, 2020 •
edited
Loading

Addresses issue #499.

agarny added 28 commits June 16, 2020 17:13
@agarny
Analyser: added a test for an empty model.
efa318b
@agarny
Make ClangFormat happy.
a27df70
@agarny
Some minor cleaning up.
d183e7e
@agarny
Analyser: test for an initialised VOI.
b8868ae
@agarny
Merge branch 'develop' into issue499.
6a4499c
@agarny
Merge branch 'develop' into issue499.
e0248f4
@agarny
Some minor cleaning up.
5ff80f8
@agarny
Analyser: make sure that a model is valid before processing it.
a64d1ac
@agarny
Analyser: make our emptyModel pass.
c2409b1 
Our empty model wasn't valid CellML...!
@agarny
Analyser: moved all/most of the analysis code from the Generator clas…
d64ccb9 
…s to the Analyser class.
@agarny
Make ClangFormat happy.
3e91cae
@agarny
Debug: have printAst() now use of our new AnalyserEquationAst class.
0f3bb44
@agarny
Generator: removed the emptyModel() and `initializedVariableOfInteg…
77b057e 
…ration()` tests.

... since they are now part of our Analyser tests.
@agarny
Analyser: moved some Generator tests that are actually analysis tests.
fa749da
@agarny
Some minor cleaning up.
b018d1b
@agarny
Analyser: added an AnalyserModel class.
e66a48e
@agarny
Some minor cleaning up.
f2fbb8b
@agarny
Some minor cleaning up.
5fc50a6
@agarny
Some minor cleaning up.
1e1e25c
@agarny
Some minor cleaning up.
d036575
@agarny
Generator: don't use internal data.
8db6248
@agarny
Some minor cleaning up.
31af8f6
@agarny
Analyser variable: provide index value.
9386de0
@agarny
Generator: don't use internal data.
a94c37a
@agarny
Some minor cleaning up.
1fc4578
@agarny
Some minor cleaning up.
5d24b9f
@agarny
Generator: created and use the GeneratorEquation class.
2e1976e
@agarny
Debug: removed printAst().
62100d6 
This will be moved to `AnalyserEquationAst`.
@agarny agarny requested a review from kerimoyle September 28, 2020 22:25
kerimoyle
kerimoyle previously approved these changes Sep 28, 2020
View reviewed changes
@agarny agarny requested a review from kerimoyle September 29, 2020 03:26
@agarny
Copy link
Contributor Author

agarny commented Sep 29, 2020

@hsorby, @kerimoyle and @nickerso: back on track and therefore ready for re^n-review.

@nickerso nickerso mentioned this pull request Sep 29, 2020
Open
nickerso
nickerso previously approved these changes Sep 29, 2020
View reviewed changes
Copy link
Contributor

@nickerso nickerso left a comment

Choose a reason for hiding this comment

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

Some minor comments, but mostly can be left to future issues to resolve. API is looking reasonable (apart from names, as per #722) and good to get this one merged in.

src/api/libcellml/analyserequation.h Show resolved Hide resolved
src/api/libcellml/analyserequationast.h Show resolved Hide resolved
src/api/libcellml/analyserequationast.h Outdated Show resolved Hide resolved
src/api/libcellml/analyserequationast.h Show resolved Hide resolved
src/api/libcellml/analysermodel.h Show resolved Hide resolved
src/api/libcellml/analysermodel.h Show resolved Hide resolved
src/api/libcellml/analysermodel.h Show resolved Hide resolved
src/api/libcellml/analyservariable.h Show resolved Hide resolved
src/api/libcellml/analyservariable.h Show resolved Hide resolved
src/api/libcellml/analyservariable.h Show resolved Hide resolved
@agarny agarny requested a review from nickerso September 29, 2020 21:38
@agarny
Copy link
Contributor Author

agarny commented Sep 29, 2020

Ok, I believe I have addressed what I could easily/quickly addressed from @nickerso's comments. So, ready for re-reviewing @hsorby, @kerimoyle and @nickerso. Would be nice to have this one in today so that you guys can then focus on PR #650. :)

@agarny agarny linked an issue Sep 29, 2020 that may be closed by this pull request
Refactoring: extract the model analysis from the code generator #499
Closed
@agarny agarny merged commit 5801557 into cellml:develop Sep 30, 2020
@agarny agarny deleted the issue499 branch September 30, 2020 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kerimoyle kerimoyle kerimoyle approved these changes

@nickerso nickerso nickerso approved these changes

@hsorby hsorby Awaiting requested review from hsorby

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactoring: extract the model analysis from the code generator
4 participants
@agarny @hsorby @kerimoyle @nickerso