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

farrayPtr2DCpy (a.k.a. arrayDup) is not initialized in ESMF_ArrayCreateGetUTest.F90 #288

Closed
danrosen25 opened this issue Sep 5, 2024 · 2 comments · Fixed by #289
Closed
Assignees

Comments

@danrosen25
Copy link
Member

if (abs(farrayPtr2D(i,j)-farrayPtr2DCpy(i,j)) < 1.d-10) dataCorrect=.false.

Test Failure

Verify Array vs Array Copy (ALLOC) no data copy, ESMF_ArrayCreateGetUTest.F90, line 411:  Unexpected data copy
if (abs(farrayPtr2D(i,j)-farrayPtr2DCpy(i,j)) < 1.d-10) dataCorrect=.false.

Initialization for farrayPtr2DCpy never happens because it is created with ESMF_DATACOPY_ALLOC
arrayDup = ESMF_ArrayCreate(array, datacopyflag=ESMF_DATACOPY_ALLOC, rc=rc)
Initialization for farrayPtr2D
farrayPtr2D = real(localPet+10, ESMF_KIND_R8)

This causes intermittent failures

@danrosen25
Copy link
Member Author

danrosen25 commented Sep 5, 2024

@theurich @oehmke
How should we fix this test? It's only guaranteed to be successful if ifort: -init=keyword [, keyword ] or gfortran: -finit-real=<zero|inf|-inf|nan>, etc are used, which we don't set for any build configuration.

We could

  • eliminate this test
  • redo the infrastructure to include a macro for #ifdef ESMF_INIT_REAL and fence this test and add the above flags for BOPT=g
  • initialize arrayDup, which defeats the purpose of the test
  • change the values in farrayPtr2D to something more obscure but this won't guarantee that the test works
  • change the test so that it checks if every array value is identical

@danrosen25 danrosen25 self-assigned this Sep 5, 2024
@theurich
Copy link
Member

theurich commented Sep 5, 2024

I agree that this test is fundamentally problematic, and can only be guaranteed 100% when using compiler level initialization as you point out. However, that gets tricky and requires maintenance when supporting many different compilers, and their versions... which is why I would like to stay away from that.

I think we could make the test more robust (not 100% but better than now) by (1) more obscure init data (as you suggest), and (2) turning around the verification logic. Currently the logic sets dataCorrect=.false. when a single element matches between the two arrays. This can happen randomly due to the uninitialized data in arrayDup. However, if we change it to set dataCorrect=.true. as soon as a single element is different (which means it is NOT an array data copy), then false positives (or negatives which ever you think about it) become much less likely, especially for a large enough array size.

danrosen25 added a commit to danrosen25/esmf that referenced this issue Sep 5, 2024
* fill array with huge - localpet
* fail if every element in array matches
danrosen25 added a commit to danrosen25/esmf that referenced this issue Sep 6, 2024
* fill array with random number 0 - 1000
* fail if every element in array matches
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 a pull request may close this issue.

2 participants