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

fix unassociated pointer #1001

Merged

Conversation

DeniseWorthen
Copy link
Contributor

@DeniseWorthen DeniseWorthen commented Apr 24, 2023

Pull Request Summary

Check if pointer is associated before use.

Description

Issue(s) addressed

Commit Message

Check if irqrs is associated before use in w3iorsmd.F90

Check list

Testing

  • How were these changes tested?
  • Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)
  • Have the matrix regression tests been run (if yes, please note HPC and compiler)?
  • Please indicate the expected changes in the regression test output, (Note the list of known non-identical tests.)
  • Please provide the summary output of matrix.comp (matrix.Diff.txt, matrixCompFull.txt and matrixCompSummary.txt):

@MatthewMasarik-NOAA
Copy link
Collaborator

Thank you catching this @DeniseWorthen!! I'll start the regtests

Copy link
Collaborator

@MatthewMasarik-NOAA MatthewMasarik-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review Pass

Regtests Pass
Only the known non-b4b's for non-identicals.

**********************************************************************                            
********************* non-identical cases ****************************                            
**********************************************************************                            
mww3_test_03/./work_PR3_UNO_MPI_e                     (1 files differ)                            
mww3_test_03/./work_PR2_UQ_MPI_e                     (1 files differ)                             
mww3_test_03/./work_PR2_UNO_MPI_d2                     (16 files differ)                          
mww3_test_03/./work_PR1_MPI_d2                     (12 files differ)                              
mww3_test_03/./work_PR3_UNO_MPI_d2_c                     (14 files differ)                        
mww3_test_03/./work_PR3_UQ_MPI_d2_c                     (16 files differ)                         
mww3_test_03/./work_PR3_UNO_MPI_d2                     (10 files differ)                          
mww3_test_03/./work_PR2_UQ_MPI_d2                     (15 files differ)                           
mww3_test_03/./work_PR3_UQ_MPI_e                     (1 files differ)                             
mww3_test_03/./work_PR3_UNO_MPI_e_c                     (1 files differ)                          
mww3_test_03/./work_PR3_UQ_MPI_d2                     (16 files differ)                           
ww3_ta1/./work_UPD0F_U                     (0 files differ)                                       
ww3_tp2.10/./work_MPI_OMPH                     (7 files differ)                                   
ww3_tp2.16/./work_MPI_OMPH                     (4 files differ)                                   
ww3_ufs1.3/./work_a                     (3 files differ)                                          
                                                                                                  
**********************************************************************                            
************************ identical cases *****************************                            
**********************************************************************

matrixCompSummary.txt
matrixDiff.txt
matrixCompFull.txt

Approved.

@MatthewMasarik-NOAA
Copy link
Collaborator

Thanks for your continued great work @DeniseWorthen.

@MatthewMasarik-NOAA MatthewMasarik-NOAA merged commit 0392615 into NOAA-EMC:develop Apr 25, 2023
@MatthewMasarik-NOAA
Copy link
Collaborator

@DeniseWorthen I wanted to check with you regarding getting this into dev/ufs-weather-model. We can get a PR going quickly for dev/ufs-weather-model if desired. Please let us know

@aronroland
Copy link
Collaborator

@DeniseWorthen this is a great catch, can u provide some details on how u did catch that one? Did it just pop up in this flags and constellation or what was your strategy?

@DeniseWorthen
Copy link
Contributor Author

DeniseWorthen commented Apr 25, 2023

@aronroland We have regression tests for UFS in debug mode for both Intel and GNU (GNU is only on two of our platforms though). Our experience is that GNU will often catch errors that Intel does not, which is one of the primary reasons we test w/ GNU. This was simply the case of compiling with our debug GNU flags and getting a run-time error about an unassociated pointer.

@DeniseWorthen
Copy link
Contributor Author

@MatthewMasarik-NOAA Regarding the update to dev/ufs-weather-model, I checked during the morning CM tagup. There are no upcoming PRs that will not also change baselines through next week. I can commit the fix though when I open the PR for the unstructured mesh, though I realize that is not the preferred thing.

@aronroland
Copy link
Collaborator

aronroland commented Apr 25, 2023

@DeniseWorthen , well yes this is the right way to proceed on our end thomas and mathieu r working a lot with gnu. I am doing intel as this goes mostly into production and so it slipped, but i will need to double check with GNU. We should design a standard procedure using both compilers with dedicated flags in terms of future pull requests.

@aliabdolali
Copy link
Contributor

WW3 code management at my time was solely with intel as some of the standalone tests failed with gnu for the sake of libraries on the RDHPC. Ifremer used to test everything with GNU, so we usually got feedback from them on GNU. It might be the time to think about multiple platforms and multiple compilers, ...
As we move towards more ONR tests in our development, we need to consider it. Thanks @aronroland and @DeniseWorthen for bringing it up. FYI: @thesser1

@aronroland
Copy link
Collaborator

@aliabdolali yeah ... we will standardise...we must ...

@MatthewMasarik-NOAA
Copy link
Collaborator

@MatthewMasarik-NOAA Regarding the update to dev/ufs-weather-model, I checked during the morning CM tagup. There are no upcoming PRs that will not also change baselines through next week. I can commit the fix though when I open the PR for the unstructured mesh, though I realize that is not the preferred thing.

@DeniseWorthen Yes, to stick to our procedure's I'll start on a PR to dev/ufs-weather-model.

@DeniseWorthen
Copy link
Contributor Author

For UFS, GNU debug compile produces these flags for WW3 code:

-g -O0 -fno-unsafe-math-optimizations -frounding-math -fsignaling-nans -ffpe-trap=invalid,zero,overflow -fbounds-check -Jmod -g -fno-second-underscore -ffree-line-length-none -Wall -fcheck=all -ffpe-trap=invalid,zero,overflow -frecursive -fbacktrace -c

MatthewMasarik-NOAA added a commit to MatthewMasarik-NOAA/WW3 that referenced this pull request Apr 26, 2023
* develop:
  Check if irqrs is associated before use in w3iorsmd.F90 (NOAA-EMC#1001)
  Fix 0 files differ and ww3_from_ftp.sh file version string (NOAA-EMC#992)
@DeniseWorthen
Copy link
Contributor Author

@MatthewMasarik-NOAA Will you or Jessica be making a PR to update dev/ufs-weather-model in UFS?

@JessicaMeixner-NOAA
Copy link
Collaborator

@MatthewMasarik-NOAA will be making this PR to update dev/ufs-weather-model in UFS.

@MatthewMasarik-NOAA
Copy link
Collaborator

Hi @DeniseWorthen, I've got a PR in progress. It will be posted shortly this afternoon.

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.

unassociated pointer when compiling with GNU in debug mode with domain decomposition
5 participants