-
Notifications
You must be signed in to change notification settings - Fork 64
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
Add debug switch to capgen (perform variable allocation checks etc) + fix spelling: var_compatability --> var_compatibility #512
Add debug switch to capgen (perform variable allocation checks etc) + fix spelling: var_compatability --> var_compatibility #512
Conversation
…ute into Fortran conditional (metavar.py)
…eme calls (suite_objects.py)
…gen_test, add --debug flag to capgen and update test reports
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self-review
c2db64d
to
681bc4f
Compare
…e_objects.py; remove pointer attribute in metavar.py and fortran_tools/parse_fortran.py; import Fortran conditional regex statements from parse_tools
681bc4f
to
af16935
Compare
…ug_check in scripts/suite_objects.py
… into feature/capgen_debug_checks
@dustinswales @peverwhee @gold2718 I cleaned this up as discussed, it's now ready for review. I merged feature/capgen in cleanly after the constituents PR was merged and reran all the tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@climbfuji Looks good. Great job!
…be consistent with how the DEBUG checking is implemented in PR NCAR#512
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple small comments, but nothing worthy of holding up the PR
Formatting Co-authored-by: mwaxmonsky <137746677+mwaxmonsky@users.noreply.github.com>
…heck to 'internal_var' variables, and avoid promoting these variables to the module (suite) level
…for variables local to the group and assign correct dimensions for local (group) and module (suite) variables
@dustinswales @peverwhee @gold2718 After much debugging (or rather staring at the code and weeding through layers and layers of
I added inline comments that explain the above in commit 7367f98. All doctstring tests and all capgen tests pass. |
Pinging all remaining reviewers. This PR has sufficient approvals and we are planning to merge this PR on Wednesday, January 17 2024. |
I am going to pull in feature/capgen today, resolve the conflicts, rerun all the tests and then merge unless someone requests changes today. |
@dustinswales Can you please re-review the changes that I made to fix the spelling |
Update 2024/01/18: when pulling in
feature/capgen
, I noticed a spelling mistake that I corrected in commit 70ae2b0:var_compatability
-->var_compatibility
. I reran all the tests after pulling infeature/capgen
and correcting the spelling, and they all passed.This PR adds a
--debug
switch to capgen which enables checking of variables.This feature is necessary for the transition from
ccpp_prebuild.py
tocapgen.py
. The following checks are performed:User interface changes?: Yes
capgen now accepts an additional (but optional) argument
--debug
This PR resolves #325. After adding the unit tests (see above), also #433
Testing:
test removed: none
unit tests: ran all unit tests in
test/unit_tests
system tests:
- ran all Fortran tests in
test/{advection_test,capgen_test,var_action_test}/
(note that the latter fail due to the missing unit conversions, but the new variable debug checks work as expected)- ran doctests in
scripts
on all files modified- ran hashtable tests in
test/hashtable
manual testing: n/a