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

make variables using filename_base consistant in length #43

Merged
merged 3 commits into from
Jan 15, 2020
Merged

make variables using filename_base consistant in length #43

merged 3 commits into from
Jan 15, 2020

Conversation

jedwards4b
Copy link

filename_base was declared with length 255, however it was then used with variables of length 80 creating an error that was difficult to track down. This makes the length of these variables consistant, it is needed for the proper naming of CIME testcase files. See issue ufs-community/ufs-mrweather-app#49

fv3_cap.F90 Outdated
@@ -74,7 +75,7 @@ module fv3gfs_cap_mod

type(ESMF_GridComp) :: fcstComp
type(ESMF_State) :: fcstState
character(len=80), allocatable :: fcstItemNameList(:)
character(len=max_filename_len+14),allocatable :: fcstItemNameList(:)
Copy link
Collaborator

Choose a reason for hiding this comment

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

How is the "14" defined? This could be confusing and error-prone when the interpolation method is changed, such as "first_order_conservative". It seems a temporary fix, from that point of view, I would suggest just setting the length of all the character strings to "ESMF_MAXSTR". Not sure the reason to have the filenames exactly 255 characters. If we do want that, we need a different fix. Basically the filenames are the local file names, if developers want to write to a different directory, they can set that up in script level.

Copy link
Author

Choose a reason for hiding this comment

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

agreed and done.

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Jan 15, 2020 via email

@DusanJovic-NOAA
Copy link
Collaborator

@jedwards4b, I'm getting the syntax error:

Scanning dependencies of target io                                               
[ 75%] Building Fortran object FV3/io/CMakeFiles/io.dir/module_fv3_io_def.F90.o  
/scratch2/NCEPDEV/fv3-cam/Dusan.Jovic/ufs/public/ufs-weather-model/FV3/io/module_fv3_io_def.F90(10): error #5082: Syntax error, found '::' when expecting one of: => ( :
  use esmf, only    :: esmf_maxstr                                               
--------------------^

The line 10 in module_fv3_io_def.F90 should be:

diff --git a/io/module_fv3_io_def.F90 b/io/module_fv3_io_def.F90
index 74b5b73..6d70681 100644
--- a/io/module_fv3_io_def.F90
+++ b/io/module_fv3_io_def.F90
@@ -7,7 +7,8 @@
 !
 !------------------------------------------------------------------------
 !
-  use esmf, only    :: esmf_maxstr
+  use esmf, only : esmf_maxstr
+
   implicit none
 !
   integer           :: num_pes_fcst

Please update the code and push to this branch.
Thanks.

@jedwards4b
Copy link
Author

Sorry - it's fixed now.

Jim

@DusanJovic-NOAA DusanJovic-NOAA merged commit 1e3cdc1 into NOAA-EMC:ufs_public_release Jan 15, 2020
@jedwards4b jedwards4b deleted the jedwards/fix_filename_length_issue branch January 15, 2020 14:53
@jedwards4b
Copy link
Author

@climbfuji Do I need to take further action to get the ufs-weather-model updated?

@DusanJovic-NOAA
Copy link
Collaborator

@jedwards4b You can submit a ufs-weather-model (ufs_public_release branch) PR with fv3atm submodule pointing to 1e3cdc1

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Jan 15, 2020 via email

@jedwards4b
Copy link
Author

jedwards4b commented Jan 15, 2020 via email

@junwang-noaa
Copy link
Collaborator

junwang-noaa commented Jan 15, 2020 via email

@jedwards4b
Copy link
Author

As I said I am happy to run tests but you still haven't told me what tests or how to run them.
I have submitted PR ufs-community/ufs-weather-model#28 to bring this to the ufs-weather-model repository.

@climbfuji
Copy link
Collaborator

I am happy to run the tests on Cheyenne tonight and send @DusanJovic-NOAA the logs for updating the ufs-weather-model repo. Back to Boulder next week I am happy to "teach" folks how to run the regression tests on Cheyenne (straightforward as long as no new baselines are required, otherwise I will need to do that).

@jedwards4b
Copy link
Author

@climbfuji This change which is critical for cime testing does not seem to be in 8394f36 of ufs-weather-model. This is why cime testing is failing on cheyenne.

@climbfuji
Copy link
Collaborator

I just checked the latest version of the release,

commit 4a1e67e86a7c316425b39a4c5668c5cc4ed110c7 (HEAD -> release/public-v1, tag: ufs-v1.0.0.beta01, origin/release/public-v1)

which has

heinzeller-lt:ufs-weather-model-ufs-public-release-20200208 dom.heinzeller$ grep -Rie 'fcstItemNameList' * | grep character
FV3/io/module_wrt_grid_comp.F90:      character(len=esmf_maxstr),allocatable    :: fcstItemNameList(:)
FV3/fv3_cap.F90:  character(len=esmf_maxstr),allocatable :: fcstItemNameList(:)

@jedwards4b
Copy link
Author

Sorry - my fault, my update failed to recurse into subdirectories - but now I get another error:

error: Server does not allow request for unadvertised object db5b3b035cc022619b2b049fd87bd63dcd3dd9d7
Fetched in submodule path 'FV3/atmos_cubed_sphere', but it did not contain db5b3b035cc022619b2b049fd87bd63dcd3dd9d7. Direct fetching of that commit failed.

@climbfuji
Copy link
Collaborator

I would start from a fresh checkout ... github probably upset by the change in branch names (or you need to do recursive git submodule sync)

@jedwards4b
Copy link
Author

it was actually the change in the remote source of atmos_cubed_sphere - fixed now.

@climbfuji
Copy link
Collaborator

Hopefully this fixes the runtime error on Cheyenne! I would like to move (back) to Intel 19.0.2 with the next version of the NCEPLIBS. Currently we are on alpha01, hope that the next version will be ready for being tagged as beta01.

@climbfuji
Copy link
Collaborator

If this fixes the issue on Cheyenne, can you please update the folks who were on the call just beforehand so that they are not under the assumption that the code is broken?

@jedwards4b
Copy link
Author

If you are going to update compilers on cheyenne please consider intel/19.0.5 and gnu/9.1.0, yes I will update this mornings call attendees as soon as I complete testing.

@climbfuji
Copy link
Collaborator

Sure - 19.1.0.x was my target.

@jedwards4b
Copy link
Author

@climbfuji I believe I now have the latest code in ~jedwards/sandboxes/mrweather.feb10/
I am still getting runtime failures from all tests.

@jedwards4b
Copy link
Author

Lets continue this discussion here: ufs-community/ufs-mrweather-app#84

climbfuji added a commit to climbfuji/fv3atm that referenced this pull request May 5, 2020
…hwrf-physics

dtc/hwrf-physics: HWRF Ferrier-Aligo MP scheme updates
DusanJovic-NOAA pushed a commit to DusanJovic-NOAA/fv3atm that referenced this pull request May 5, 2020
make variables using filename_base consistant in length
DusanJovic-NOAA added a commit that referenced this pull request May 8, 2020
climbfuji pushed a commit to climbfuji/fv3atm that referenced this pull request May 8, 2020
climbfuji pushed a commit to climbfuji/fv3atm that referenced this pull request Aug 10, 2020
…_develop

Update gsd/develop from NOAA-EMC develop
@joeolson42 joeolson42 mentioned this pull request Mar 1, 2023
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.

4 participants