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

More testing on addfield.f #520

Merged
merged 9 commits into from
Aug 9, 2023

Conversation

kingofnart
Copy link
Contributor

Part of #377

Added a test for errror cases 3 and 6 in addfield.f

@kingofnart kingofnart marked this pull request as draft August 2, 2023 01:44
@edwardhartnett
Copy link
Contributor

Best hope for debugging memory errors is firstly to comment out as much of the test code as you need to so that the test passes.

Then uncomment the smallest piece of test code which causes the memory error and debug that...

@edwardhartnett
Copy link
Contributor

Please comment out whatever code is causing the memory issues so we can get the rest of the test merged and start working on the memory problems. It's hard for me to help when this is on a branch of your fork.

@kingofnart
Copy link
Contributor Author

The issue I'm having is that all the tests pass on my machine. It looks like the last error had to do with the simpack call on line 289 of addfield.f but I can't reproduce the error locally.

@edwardhartnett edwardhartnett marked this pull request as ready for review August 9, 2023 10:10
@edwardhartnett
Copy link
Contributor

I'm going to merge what you;'ve got here and work on it in a separate PR to get the rest of the test code working...

@edwardhartnett edwardhartnett merged commit 66d1532 into NOAA-EMC:develop Aug 9, 2023
26 checks passed
@edwardhartnett
Copy link
Contributor

OK, there are two different ways we run memory tests in the CI system, and either can be done on your machine and should reproduce the problem.

The one that is failing is the address sanitizer. In order to use the address sanitizer we add the following flags to our C flags and our Fortran flags:
-fno-omit-frame-pointer -fsanitize=address

You can reproduce this in your own cmake build by adding to the cmake command line:

cmake -DCMAKE_C_FLAGS="-fno-omit-frame-pointer -fsanitize=address" -DCMAKE_Fortran_FLAGS="-fno-omit-frame-pointer -fsanitize=address" -DCMAKE_BUILD_TYPE=Debug etc.

So give it a try...

@edwardhartnett
Copy link
Contributor

Also for examples of how to run the memchecks or anything else look in file .github/workfows/developer.yml.

@edwardhartnett
Copy link
Contributor

OK, there was only one problem with this code, which was that the real array was declared real*4. Once the *4 was removed the test works.

The reason is that we build with -d option as well, which has default real as 8-byte, unless you explicitly set the size. So when you set it to *4 it is *4 even on *8 builds, and breaks. When we leave it unset, it is *4 on *4 builds, and *8 on *8 builds.

See #530 for the working test code. Thanks for this test it is very useful.

@kingofnart
Copy link
Contributor Author

Oh that's right I remember disabling ASAN to use valgrind for debugging but I never added the -fsanitize=address flags back. Ok, that makes sense why it was only the _d test that failed then.

@kingofnart kingofnart deleted the ask8-1_testing_addfield branch August 9, 2023 16:40
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.

2 participants