convert remaining IN and OUT test codes to F90 #331
Conversation
|
So I added a new outtest8.F90 test to capture some more of the library subprograms that weren't yet being tested. And I correspondingly updated the bufr-11.6.0.tgz bundle on the emcrzdm server to include the input and baseline output files for this new test. Everything works fine when I run this locally in my own area and let it download the updated bufr-11.6.0.tgz bundle from the server. But the automated CI tests apparently don't download this bundle and instead copy it in from a stored cache, according to the @edwardhartnett how do we replace the old bufr-11.6.0.tgz in /home/runner/data with the new one? Until we do this, the cmake option |
|
I know I have admin access to this repository, but I've been through all of the Settings pages and don't see any way to update the cache with the new bufr-11.6.0.tgz file. @edwardhartnett once you're back from leave, I'd really appreciate a quick tutorial to understand how you cached that file in the first place, and how to update it (thanks ;-) |
|
OK, I didn't notice until now that you had converted the rest of the IN tests. So I have done some in another PR. In order to invalidate the data cache in the develop branch change this line in developer.yml:
|
| ! Scan for certain values across all of the data subsets in the internal arrays, and verify some of them. | ||
| call ufbtam ( r8vals, mxr8pm, mxr8lv, nsub, 'CLAT CLON' ) | ||
| if ( ( nsub .ne. 18447 ) .or. & | ||
| ( nint(r8vals(1,1285)*100) .ne. 4328 ) .or. ( nint(r8vals(2,1285)*100) .ne. -7910 ) .or. & |
There was a problem hiding this comment.
All the excess parens are not necessary.
There was a problem hiding this comment.
I agree they're not needed, but IMHO it's better for readability to have those extra parentheses surrounding each logical operator in the expression, and which is why I usually write them that way.
test/intest7.F90
Outdated
| iret1 = igetprm ( 'MXNRV' ) | ||
| errstr_len = 1 | ||
| iret2 = igetprm ( 'DUMMY' ) | ||
| if ( ( iret1 .ne. 5 ) .or. ( iret2 .ne. -1 ) .or. & |
There was a problem hiding this comment.
Check results as soon as you get them. So test iret1 right after linke 74 instead of waiting to call igetprm() again.
It should be generally the case that after every call to the library in a test, you should test every value that can be tested. before calling another library function.
There was a problem hiding this comment.
OK, good suggestion, and will do
test/intest7.F90
Outdated
| call ufbrep ( 11, r8val, 0, 1, nr8v2, 'TIDER' ) | ||
| idx2 = index( errstr(1:errstr_len), 'UFBREP - 3rd ARG. (INPUT) IS .LE. 0' ) | ||
| if ( ( nr8a .ne. 2 ) .or. ( nr8v .ne. 0 ) .or. ( nr8v2 .ne. 0 ) .or. ( idx1 .eq. 0 ) .or. ( idx2 .eq. 0 ) & | ||
| .or. ( nint ( r8arr(1,1) ) .ne. -10000 ) .or. ( nint ( r8arr(1,2) ) .ne. 16 ) ) stop 4 |
There was a problem hiding this comment.
Some of these values like r8arr can be checked before later library calls. If they are incorrect, we want to exit immediately, not run additional library code.
There was a problem hiding this comment.
Again, good suggestion, and I'll make a quick pass through all of the intest codes to clean these up. That will probably allow us to reduce the number of declared variables as well.
There was a problem hiding this comment.
Feel free to merge this PR and then do variable consolidation in another iteration...
There was a problem hiding this comment.
Thanks, but I'm going to have to push up another commit anyway to fix the key setting in developer.yml, so I may as well do all of this together ;-)
| print *, 'Testing writing OUT_3 using ISETPRM and IGETPRM, using EXITBUFR with multiple allocations,' | ||
| print *, 'and using 2-22, 2-36 and 2-37 operators.' | ||
|
|
||
| #ifdef KIND_8 |
There was a problem hiding this comment.
Are there any tests where we have to change the settings for KIND_d?
There was a problem hiding this comment.
No, because both _d and _4 use 4-byte integers.
The only difference in these two builds is that the former uses 8-byte reals instead of 4-byte reals. But unless we missed something, every real call parameter is already explicitly passed as real*8 anyway in the library specs, so the two builds are now basically equivalent from a library interface perspective.
|
BTW, thanks for the explanation of how to force the runner to change the data in the cache! I'll take care of that when I push up the next commit. I may still have one new intest code to add if I can get my hands on a sample prepfits file, and in which case I can cook up a test for the ufbin3 routine as well. I'll keep you posted. |
|
BTW, I'm guessing I'm going to need to update that key setting in all of the .yml files, not just in developer.yml, correct? At the moment, we have the following in each of the .yml files:
So MacOS is already pointing to a different data-2 key - do you have any idea why? And does this mean that I should now update all of them to be |
|
Just increment all the keys by 2 and that should work. MacOS has a different cache from Linux, IIRC. |
…ache, across all yml workflows
Fixes #308
Fixes #295
Part of #33