esmf-org / esmf

The Earth System Modeling Framework (ESMF) is a suite of software tools for developing high-performance, multi-component Earth science modeling applications.
https://earthsystemmodeling.org/
Other
156 stars 75 forks source link

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

Closed danrosen25 closed 1 week ago

danrosen25 commented 1 month ago

https://github.com/esmf-org/esmf/blob/82ba326579f89b8946c99b2f953d37223b28d7e0/src/Infrastructure/Array/tests/ESMF_ArrayCreateGetUTest.F90#L408

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 commented 1 month ago

@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

theurich commented 4 weeks ago

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.