Skip to content

Comments

Remove standard_wrf_dirs and use wildcard for WRF paths#248

Open
IncubatorShokuhou wants to merge 2 commits intowrf-model:developfrom
IncubatorShokuhou:standard_wrd_dir
Open

Remove standard_wrf_dirs and use wildcard for WRF paths#248
IncubatorShokuhou wants to merge 2 commits intowrf-model:developfrom
IncubatorShokuhou:standard_wrd_dir

Conversation

@IncubatorShokuhou
Copy link

In this PR, I removed the standard_wrf_dirs variable and switched to using regular expressions, so we don't need to modify the WRF directory name or update the standard_wrf_dirs variable every time a new version is released.

@mgduda mgduda changed the base branch from master to develop May 10, 2024 16:11
@mgduda mgduda requested review from islas and mgduda May 10, 2024 16:15
@islas
Copy link
Contributor

islas commented May 10, 2024

-maxdepth nor -regex are standard to find
An alternative could be :
ls .. | grep -E 'WRFV?(-?[0-9]+(\.[0-9](\.[0-9])?)?)?'

However, caution should be used as both options do not appear in version order. The naming is also variable enough that POSIX sort cannot work well to put these in order.

@IncubatorShokuhou
Copy link
Author

-maxdepth nor -regex are standard to find An alternative could be : ls .. | grep -E 'WRFV?(-?[0-9]+(\.[0-9](\.[0-9])?)?)?'

However, caution should be used as both options do not appear in version order. The naming is also variable enough that POSIX sort cannot work well to put these in order.

I have abandoned the use of complex regular expressions and have instead opted for more explicit if statements. This should be more convenient for maintenance.

@islas
Copy link
Contributor

islas commented May 13, 2024

-V is part of GNU sort, so I would recommend something like:
sort -t. -k 1,1.4n -k 2,2nr -k 3,3nr
to get the desired effect. The breakout into explicit if statements does look a lot more manageable.

@mgduda
Copy link
Collaborator

mgduda commented May 17, 2024

-V is part of GNU sort, so I would recommend something like: sort -t. -k 1,1.4n -k 2,2nr -k 3,3nr to get the desired effect. The breakout into explicit if statements does look a lot more manageable.

Should this be

sort -t. -k 1.4,1n -k 2,2nr -k 3,3nr

instead?

@islas
Copy link
Contributor

islas commented May 17, 2024

You're right, I confused the first and last character placement

@mgduda
Copy link
Collaborator

mgduda commented May 17, 2024

I'm not sure whether the incorporation of the - character in the first key is too clever or not. So an alternative might be

sort -t. -k 1.5,1nr -k 2,2nr -k 3,3nr

This may be a bit clearer, since all keys are being sorted in reverse numerical order.

WRF_DIR="../WRF"
else
# If the WRF directory does not exist or libwrfio_nf.a is not found, search for directories starting with WRF-4 in descending order of version number
for dir in $(ls .. | grep '^WRF-4' | sort -rV); do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Following the discussion on this PR, I'd suggest we change sort -rV to sort -t. -k 1.5,1nr -k 2,2nr -k 3,3nr.

# find WRF dir
if [ -e "../WRF/external/io_netcdf/libwrfio_nf.a" ]; then
echo "Found what looks like a valid WRF I/O library in ../WRF"
WRF_DIR="../WRF"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, as well as on lines 212 and 219, I think WRF_DIR should be wrf_dir; otherwise, the check on line 235 will fail.

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.

3 participants