Add array checking support for `check` interface

[zoziha]

trafficstars

Description

• [x] Add rank-1 arrays support for check interface;
• [x] Add new tests;
• [x] Add the pure keyword to pure functions.

Method: In order to avoid introducing double the number of check interfaces in testdrive for array checking, this PR uses class(*) to characterize the parameters. (+238 lines of code)

~~To close #19 , array checking is supported in this PR, but two problems are found, mainly in gfortran <= 10,~~

• ~~Problem 1: gfortran <= 10 seems to have limitations in the support of class(*) and associate(pointer => array);~~
• ~~Problem 2: The reshape of gfortran <= 10 may have adaptability problems with class(*).~~

~~After some adjustments, the above two problems are avoided, the content of this PR can theoretically pass gfortran >= 7 compilers. This means that if we want to support array checking and there is no better way, we need to remove support for gfortran 6 and add support for gfortran 12.~~

~~Help: But I can't solve the problem with cmake and meson config files, their CI always tells me that something went wrong, hope to get help, @awvwgk .(Solved)~~

[awvwgk]

The select type logic is not really that much shorter than having several duplicated procedures for dealing with one-dimensional arrays.

The test failures might be related to a routine which is guarded in the fpm case (WITH_XDP and WITH_QP) as those are disabled by default, while in meson / CMake they are enabled if compiler support is available.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 59.11330% with 83 lines in your changes missing coverage. Please review.

Project coverage is 66.41%. Comparing base (dc60eaf) to head (986fa4f). Report is 12 commits behind head on main.

Files with missing lines	Patch %	Lines
src/testdrive.F90	59.11%	0 Missing and 83 partials :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #20      +/-   ##
==========================================
- Coverage   71.51%   66.41%   -5.11%     
==========================================
  Files           2        2              
  Lines         481      655     +174     
  Branches      296      436     +140     
==========================================
+ Hits          344      435      +91     
  Misses         23       23              
- Partials      114      197      +83     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[zoziha]

The select type logic is not really that much shorter than having several duplicated procedures for dealing with one-dimensional arrays.

At the beginning, I tried to implement multiple procedures for dealing with one-dimensional arrays, which probably added 600+ lines of code for test-drive.F90, I think it seems that select type may reduce the amount of code (+238 lines of code), if we worry about select type will degrade performance, or if we want to preserve gfortran 6 compatibility, we can change back to the multiple-procedure scheme.

---

By the way, now CI says: only gfortran 6 fails, codecov coverage is reduced. gfortran 6 fails, probably not handling class(*), pointer and character(*) well:

 Starting character ... (86/89)

Program received signal SIGSEGV: Segmentation fault - invalid memory reference.

Backtrace for this error:
#0  0x7f5170f1cd57 in ???
#1  0x7f5170f1bfbd in ???
#2  0x7f51707ad0bf in ???
#3  0x7f51708f5764 in ???
#4  0x406bc1 in __testdrive_MOD_check_string
	at /src/test_drive.F90:1561
#5  0x40284c in __testdrive_MOD_check_double_array
	at /src/test_drive.F90:2099
#6  0x40e02f in test_char
	at /test/test_check_array.F90:1434
#7  0x40cb26 in run_unittest
	at /test/test_drive.F90:402
#8  0x40d20f in __testdrive_MOD_run_testsuite
	at /test/test_drive.F90:346
#9  0x4202d2 in tester
	at /test/main.f90:62
#10  0x4203e5 in main
	at /test/main.f90:18

[zoziha]

@awvwgk, @jvdp1, maybe the latest commit will make us more satisfied:

1. For a single array check, select type is still used, which is concise enough and introduces a small amount of code;
2. For two array checks, multiple procedures are used, which satisfies the compatibility of gfortran 6 and increases the amount of code to a certain extent.

It would be nice to see that test-drive supports array checking. Arrays are a common data structure in Fortran. We can easily reshape any rank array into a rank-1 array for checking:

call check(error, reshape(array, [size(array)], expected, ...)

---

PS. In terms of efficiency, test-drive makes xdp and qp precision optional. Due to the addition of array checking, the check interface has become heavier. In our daily calculations, real numbers seem to be used more than integers. Do we need to set optionals for integers, like integer(i1,i2) as optionals? Of course, if we let it go, it's fine.

[zoziha]

Thank you for review. This commit has added a check on the size of the array. Since my native language is not English, if there are some mistakes in the text, please point them out.

summary:

• The first commit, adding array support, a total of +238 lines of code;
• The second commit, partially use the multi-procedures scheme, a total of +633 lines of code;
• The third commit, adding a check on the array size, a total of +701 lines of code.

(Hope it doesn't add too much code burden.)

[HugoMVale]

Hello. Just want to state that I find this new feature quite useful, actually a must. I am using it, with success, in my own code since yesterday (I was about to write something similar myself at least for dp arrays). Is there any special reason why the merge is pending? Thanks.

[awvwgk]

Is there any special reason why the merge is pending?

There is nothing fundamentally blocking this patch, if you want to move this forward, feel free to help out with the code review.

[HugoMVale]

@awvwgk, I only have a superficial understanding of the PR/review/approval process. But there is a first time for everything, and I am happy to learn. Could I ask you to point me in the right direction? How exactly can I help? How do I start the process? Thanks... and sorry for the beginner questions.

[zoziha]

My main concern is that although the current solution caters to the coding paradigm of test-dive, such a heavy API may be unsustainable in the future and destroy the simplicity of the original test-drive, but I can't think of a better way .

Not supporting arrays is fine if we stick to test-drive brevity.

[HugoMVale]

@zoziha, for sure, this new feature adds a non-negligible number of new functions and code lines, but IMO this burden is outweighed by the benefits of the feature itself. Moreover, IMO, the maintainability issue is not really due to this new addition, but something that was already there. For instance, the functions check_float_sp, check_float_dp, check_float_xdp, check_float_qp, are already "almost-perfect-copies" of each other except for 2-3 characters. Any correction/improvement will involve a lot of copy-paste... but this is not a problem of the code - it's rather Fortran's way. Regarding the code itself, some questions/suggestions:

• call check(error, size(actual), size(expected), message, more) -> call check_int_i4(error, size(actual), size(expected), message, more) ?;
• Similarly, inside each new check_type_r1 subroutine, could we not replace call check(error, actual(i), expected(i), message, more) by the specific type subroutine check_type, in order to avoid passing each time through the interface.

Note sure if this change is of any computational benefit, but it does seem more explicit.

[zoziha]

Any correction/improvement will involve a lot of copy-paste... but this is not a problem of the code - it's rather Fortran's way.

@HugoMVale, yes, I agree with you. In the existing Fortran grammar, if some generic features are to be supported, certain trade-offs are often required. Thanks for the suggestion, it is possible to use module procedures, which are more explicit, and the patch has been updated now.