NOAA-GFDL / GFDL_atmos_cubed_sphere

The GFDL atmos_cubed_sphere dynamical core code
Other
56 stars 118 forks source link

initialize two arrays and fix fortran coding error plus PRs #285 and #276 #280

Closed SamuelTrahanNOAA closed 11 months ago

SamuelTrahanNOAA commented 1 year ago

Description

279: fortran coding error: string length mismatch

Wherein a string length mismatch causes an abort. The code does not follow the fortran standard on this line:

    attrList=(/trim(axis_name),trim(axis_name)//":cartesian_axis"/)

because the two strings in the array have different lengths. See #279 for details.

281: uninitialized arrays

The srf_wnd_var2 and tracers_var3 arrays are not initialized after allocation in fv_ufs_restart_io.F90. This can cause the ufs-weather-model write component's quilting restart to fail. When ESMF copies data from compute ranks to write ranks, it'll encounter NaN values if the uninitialized array happen to contain any.

How Has This Been Tested?

ufs-weather-model regression tests.

Specifically, the gnu version of the conus13km_qr_debug regression test.

The regression test can be found in this branch. To see the failure, disable this bug fix.

https://github.com/ufs-community/ufs-weather-model/pull/1893

Checklist:

Please check all whether they apply or not

zach1221 commented 12 months ago

Hello, @lharris4 @bensonr are you able to continue reviewing this PR?

lharris4 commented 12 months ago

Hi, Zach. I hadn't realized I was a reviewer. I have approved the PR and it should be able to move forward soon.

Thanks Lucas

On Thu, Sep 21, 2023 at 3:45 PM zach1221 @.***> wrote:

Hello, @lharris4 https://github.com/lharris4 @bensonr https://github.com/bensonr are you able to continue reviewing this PR?

— Reply to this email directly, view it on GitHub https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/280#issuecomment-1730196586, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMUQRVGGEXFYLS5RGO4LSADX3SKNXANCNFSM6AAAAAA4Q56CFU . You are receiving this because you were mentioned.Message ID: @.***>

lharris4 commented 12 months ago

BTW @bensonr is on leave today ; he should be back on Monday.

SamuelTrahanNOAA commented 11 months ago

I've merged #283 into this PR since that one is expected to go in first. This is just to speed up the UFS test process.

In the unlikely event that #283 is rejected, I'll back out those changes.

EDIT: The testing of #283, at the ufs-weather-model level, as https://github.com/ufs-community/ufs-weather-model/pull/1903, is almost done.

lharris4 commented 11 months ago

Thanks for taking the lead on this. These changes should be fine.

Lucas

On Mon, Sep 25, 2023 at 1:48 PM Samuel Trahan (NOAA contractor) < @.***> wrote:

I've merged #283 https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/283 into this PR since that one is expected to go in first. This is just to speed up the UFS test process.

In the unlikely event that #283 https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/283 is rejected, I'll back out those changes.

— Reply to this email directly, view it on GitHub https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/280#issuecomment-1734205398, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMUQRVEPC2HB6HCU7R5U4JLX4G7VPANCNFSM6AAAAAA4Q56CFU . You are receiving this because you were mentioned.Message ID: @.***>

bensonr commented 11 months ago

I've merged #283 into this PR since that one is expected to go in first. This is just to speed up the UFS test process.

In the unlikely event that #283 is rejected, I'll back out those changes.

EDIT: The testing of #283, at the ufs-weather-model level, as ufs-community/ufs-weather-model#1903, is almost done.

As you've included this here, should we have @DusanJovic-NOAA close the #283?

SamuelTrahanNOAA commented 11 months ago

As you've included this here, should we have @DusanJovic-NOAA close the https://github.com/NOAA-GFDL/GFDL_atmos_cubed_sphere/pull/283?

No. That PR should be merged now.

laurenchilutti commented 11 months ago

@SamuelTrahanNOAA I have just merged PR 283 into the dev/emc branch, but this PR is still showing the changes from PR 283 in the list of changes. Could you please merge in dev/emc branch to this branch? I believe that GitHub is confusing your merge of Dusan's changes in commit 81f254d95aba6fb9ae94a18b432eb1a4be3799a8 with the actual PR 283 dev/emc merge commit.

SamuelTrahanNOAA commented 11 months ago

Could you please merge in dev/emc branch to this branch?

I have merged dev/emc to this branch. Please wait to merge my PR until the UFS code managers finish testing it at the ufs-weather-model level.

SamuelTrahanNOAA commented 11 months ago

This PR now includes #285 and #276 by request of @laurenchilutti. I've tested #285 on six platforms. I'll begin to test #276 shortly.

SamuelTrahanNOAA commented 11 months ago

Could @junwang-noaa and @laurenchilutti please confirm that your PRs were merged into this one correctly?

jkbk2004 commented 11 months ago

@laurenchilutti As @SamuelTrahanNOAA 's mini-test went ok through https://github.com/ufs-community/ufs-weather-model/pull/1893, I don't see any major issue to merge this pr. Can you go ahead to merge in this pr by the end of today?