NCAR / ccpp-framework

Common Community Physics Package (CCPP)
http://www.dtcenter.org/community-code/common-community-physics-package-ccpp/
Other
26 stars 63 forks source link

Add debug switch to capgen (perform variable allocation checks etc) + fix spelling: var_compatability --> var_compatibility #512

Closed climbfuji closed 6 months ago

climbfuji commented 9 months ago

Update 2024/01/18: when pulling in feature/capgen, I noticed a spelling mistake that I corrected in commit 70ae2b0568a3353c81cee3b72b721d353ad0c27c: var_compatability --> var_compatibility. I reran all the tests after pulling in feature/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 to capgen.py. The following checks are performed:

  1. If if the variable is not active (based on its active attribute), skip it
  2. Variables that are allocatable or are pointers (scalars, arrays) and that have intent out for a scheme are skipped
  3. For scalars that have intent in or inout, assign the current value to a dummy variable
  4. For arrays, first check the size of the array against what's expected from the metadata, then assign the lower and upper bounds to a dummy (scalar) variable

User interface changes?: Yes capgen now accepts an additional (but optional) argument --debug

This PR resolves https://github.com/NCAR/ccpp-framework/issues/325. After adding the unit tests (see above), also https://github.com/NCAR/ccpp-framework/issues/433

Testing: test removed: none unit tests: ran all unit tests in test/unit_tests system tests:

climbfuji commented 8 months ago

@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.

climbfuji commented 7 months ago

@dustinswales @peverwhee @gold2718 After much debugging (or rather staring at the code and weeding through layers and layers of find_variable, I finally understand what was wrong with my code. I just pushed https://github.com/NCAR/ccpp-framework/pull/512/commits/7367f985e54692d352aa28cb7df9f9fc0c303d21 which makes two important updates:

I added inline comments that explain the above in commit 7367f985e54692d352aa28cb7df9f9fc0c303d21. All doctstring tests and all capgen tests pass.

climbfuji commented 6 months ago

Pinging all remaining reviewers. This PR has sufficient approvals and we are planning to merge this PR on Wednesday, January 17 2024.

climbfuji commented 6 months ago

I am going to pull in feature/capgen today, resolve the conflicts, rerun all the tests and then merge unless someone requests changes today.

climbfuji commented 6 months ago

@dustinswales Can you please re-review the changes that I made to fix the spelling compatability --> compatibility after I pulled in feature/capgen? Commit https://github.com/NCAR/ccpp-framework/pull/512/commits/70ae2b0568a3353c81cee3b72b721d353ad0c27c. If you could let me know then I will do a squashed merge with a proper commit message. Thanks!