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.

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

bring over last remaining WCOSS src updates, and add new test_OUT_5 program #58

Merged
merged 9 commits into from
Nov 10, 2020

Conversation

jbathegit
Copy link
Collaborator

@jbathegit jbathegit commented Nov 9, 2020

Fixes #55
Fixes #57

These are the last remaining src updates to be migrated over to NCEPLIBS-bufr on GitHub, and they'll have no impact on any operational user codes because src/ufdump.f and src/ufbdmp.f are utilities that are only ever run off-line to generate verbose ASCII dumps of a BUFR file. In other words, these subprograms are explicitly designed and designated for use as off-line diagnostic tools to help visualize the contents of a BUFR file.

Once this is in, I promise I'll get back to focusing on documentation! ;-)

@jbathegit
Copy link
Collaborator Author

Hmm, OK looks like I had a fail in the push, even though it worked fine when I tested in my area. Let me take a closer look.

@aerorahul
Copy link
Contributor

@jbathegit
Is there a reason why ufbdmp.f and ufdump.f are not part of the library?

@jbathegit
Copy link
Collaborator Author

@aerorahul Sorry if I wasn't clear. Those subprograms are part of the library, but the point I was trying to emphasize is that they're never called within real-time/operational codes. They're only ever called within off-line diagnostic codes for users who want to print a verbose ASCII listing of the file. In fact, they can actually be called interactively to print output in chunks of 10 lines at a time.

@edwardhartnett So the issue with the automated build (which was done using Gnu) is that, apparently, when you have a line of Fortran code such as the following in src/ufdump.f:

WRITE (LUOUT, *)

which is intended to just print a blank line to the output ASCII file, it actually prints just an end-of-line character and nothing more, whereas all of my test builds (which were done using Intel) instead write a single blank space and then an end-of-line character. So basically a difference of one blank space, and I had to use ":set list" within vi to actually see what was going on here! So if we'd been using "diff -w" then it would have been fine, but of course "cmp -s" flags this difference. At any rate, I think I can fix this fairly easily in a way so that it generates consistent output all compilers, and sorry about this!

@aerorahul
Copy link
Contributor

@aerorahul Sorry if I wasn't clear. Those subprograms are part of the library, but the point I was trying to emphasize is that they're never called within real-time/operational codes. They're only ever called within off-line diagnostic codes for users who want to print a verbose ASCII listing of the file. In fact, they can actually be called interactively to print output in chunks of 10 lines at a time.

@edwardhartnett So the issue with the automated build (which was done using Gnu) is that, apparently, when you have a line of Fortran code such as the following in src/ufdump.f:

WRITE (LUOUT, *)

which is intended to just print a blank line to the output ASCII file, it actually prints just an end-of-line character and nothing more, whereas all of my test builds (which were done using Intel) instead write a single blank space and then an end-of-line character. So basically a difference of one blank space, and I had to use ":set list" within vi to actually see what was going on here! So if we'd been using "diff -w" then it would have been fine, but of course "cmp -s" flags this difference. At any rate, I think I can fix this fairly easily in a way so that it generates consistent output all compilers, and sorry about this!

write(LUOUT, *) ' ' ?

@jbathegit
Copy link
Collaborator Author

Actually, the first thing I'm going to try is using a "/" in the format to accomplish the same thing. But yes that would probably also work, so I'll try that if my first idea doesn't pan out.

@jbathegit
Copy link
Collaborator Author

OK done and fixed, now with a 2nd small add-on commit. Please review whenever you have an opportunity, and sorry again for the delay!

@jbathegit
Copy link
Collaborator Author

Friendly reminder to @edwardhartnett - could you please review this pull request when you get a chance? Or if you want I can get someone else to look it over - just let me know (and thanks!)

@jbathegit
Copy link
Collaborator Author

I will push another commit for the above requested changes. Since I'm doing another commit for this anyway, is it OK if I go ahead and include the two small changes needed for #57, so we can get that one closed as well?

@jbathegit
Copy link
Collaborator Author

OK done, so when you get a chance please (re)review! ;-)

@jbathegit
Copy link
Collaborator Author

Just realized I'd missed icbfms.f in your initial request, so just pushed up another follow-on commit for that one as well ;-)

@jbathegit jbathegit merged commit b55c73b into develop Nov 10, 2020
@jbathegit jbathegit deleted the jba_testOUT05 branch November 10, 2020 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

do we still need -DENABLE_TESTS=ON? updates to subroutines UFDUMP and UFBDMP, including new test cases
3 participants