Skip to content

Add FVCOM lake surface conditions to UFS#153

Closed
dmwright526 wants to merge 8 commits into
ufs-community:developfrom
dmwright526:feature/FVCOM
Closed

Add FVCOM lake surface conditions to UFS#153
dmwright526 wants to merge 8 commits into
ufs-community:developfrom
dmwright526:feature/FVCOM

Conversation

@dmwright526
Copy link
Copy Markdown
Collaborator

This feature takes lake surface data generated by FVCOM and
replaces variables in sfc_data.nc (created by chgres_cube) with it.
Input data into fvcom_to_FV3.exe needs to be in .nc format and
already horizontally interpolated to the destination grid used in
sfc_data.nc. Some QC is done within the code to create a minimum
lake ice value of 15%.

This code is needed to run PR #293 in the regional_workflow.

Fortran code is strongly based upon work done by Eric James (GSL)
and Ming Hu (GSL) to get FVCOM surface conditions into HRRRv4.

Testing has been conducted on Jet using Intel compiler (18.0.5.274)

This feature takes lake surface data generated by FVCOM and
replaces variables in sfc_data.nc (created by chgres_cube) with it.
Input data into fvcom_to_FV3.exe needs to be in .nc format and
already horizontally interpolated to the destination grid used in
sfc_data.nc. Some QC is done within the code to create a minimum
lake ice value of 15%.

Fortran code is strongly based upon work done by Eric James (GSL)
and Ming Hu (GSL) to get FVCOM surface conditions into HRRRv4.

Testing has been conducted on Jet using Intel compiler (18.0.5.274)
@GeorgeGayno-NOAA
Copy link
Copy Markdown
Collaborator

Sorry, I don't know who you are or what this PR is about. Who is the EMC focal point for this work?

@dmwright526
Copy link
Copy Markdown
Collaborator Author

@GeorgeGayno-NOAA This is work being done in collaboration between GLERL and GSL (@JeffBeck-NOAA) to provide lake surface conditions generated from FVCOM to future implementations of RRFS, similarly to previous work connecting FVCOM to HRRRv4.

@JeffBeck-NOAA
Copy link
Copy Markdown
Collaborator

@GeorgeGayno-NOAA, this code will be used to replace lake surface conditions in the NetCDF files created by chgres_cube. The Fortran code in this PR will be built and run in-line with the SRW App workflow, and will only be used if the user decides to implement it when they generate their experiment. Since this code constitutes a UFS pre-processing utility, it should be housed within the UFS_UTILS repository. It represents a stand-alone utility that doesn't affect any other pre-processing, and therefore won't affect or change any of the UFS_UTILS regression tests. Please feel free to contact me or @dmwright526 for any further information.

@GeorgeGayno-NOAA
Copy link
Copy Markdown
Collaborator

Could you please add a brief readme.md file for your code. What it does, who to contact with questions, etc. That sort of information. You could copy or link to the information in the main routine's prolog. Or just say 'see prolog for more information'. And include an email contact(s). Thanks.

@GeorgeGayno-NOAA
Copy link
Copy Markdown
Collaborator

We are trying to follow the convention not including the ".exe" extension on executables. Anything in ./exec is assumed to be an executable. Can you rename it or does that cause headaches for the SRW App workflow?

@dmwright526
Copy link
Copy Markdown
Collaborator Author

@GeorgeGayno-NOAA that isn't a problem. I can update the workflow for a non .exe extension and remove it from the build process.

DavidWright-NOAA and others added 2 commits September 25, 2020 20:23
Add readme.md file to describe purpose of the routine. Also removed
name extension of .exe from build process to align with other
routines.
Increase the length of the string "mosaic_name" in make_solo_mosaic.c from 
128 to 255 characters.   This is needed for the regional workflow where
"mosaic_name" includes the absolute path and can exceed 128 characters. 
In that case, make_solo_mosaic fails without a clear error message.
@GeorgeGayno-NOAA
Copy link
Copy Markdown
Collaborator

'develop' was updated last Friday. You will need to merge these changes to your branch. They should not affect what you are doing.

This feature takes lake surface data generated by FVCOM and
replaces variables in sfc_data.nc (created by chgres_cube) with it.
Input data into fvcom_to_FV3.exe needs to be in .nc format and
already horizontally interpolated to the destination grid used in
sfc_data.nc. Some QC is done within the code to create a minimum
lake ice value of 15%.

Fortran code is strongly based upon work done by Eric James (GSL)
and Ming Hu (GSL) to get FVCOM surface conditions into HRRRv4.

Testing has been conducted on Jet using Intel compiler (18.0.5.274)
Add readme.md file to describe purpose of the routine. Also removed
name extension of .exe from build process to align with other
routines.
@dmwright526
Copy link
Copy Markdown
Collaborator Author

@GeorgeGayno-NOAA, Thanks! Those changes should be merged now. Let me know if there are any other issues.

@GeorgeGayno-NOAA
Copy link
Copy Markdown
Collaborator

@GeorgeGayno-NOAA, Thanks! Those changes should be merged now. Let me know if there are any other issues.

Something is odd with your merge. The last update to develop (from Gerard) is shown as an addition. Here is how I recommend merges be done: https://github.com/NOAA-EMC/UFS_UTILS/wiki/3.-Checking-out-code-and-making-changes

@dmwright526
Copy link
Copy Markdown
Collaborator Author

@GeorgeGayno-NOAA, So I thought I had followed those steps you pointed to for my fork. When I attempt to do them again it is telling me that the develop branch of my fork is up to date. I am not sure what you mean by an "addition"...

@GeorgeGayno-NOAA
Copy link
Copy Markdown
Collaborator

@GeorgeGayno-NOAA, So I thought I had followed those steps you pointed to for my fork. When I attempt to do them again it is telling me that the develop branch of my fork is up to date. I am not sure what you mean by an "addition"...

If you click on 'files changed' (at the top), it indicates your branch is making changes to 'make_solo_mosaic.c'. Those changes should already be in 'develop'. That is odd because 'develop' is up-to-date in your fork. When I look at the recent commits to your branch, I see:

  • The addition of your new program at a94f386
  • The addition of the readme at 627c175
  • The updates from 'develop' at dbcad06.

Then I see the first two commits repeated at dd33bbf and 5b3d074. That should not be necessary.

Revert your branch back to 627c175. The retry the merge from develop. I am curious what is going on.

@dmwright526
Copy link
Copy Markdown
Collaborator Author

@GeorgeGayno-NOAA I tried it again, so hopefully this is cleaner. If not, since it does not depend on too many files, I can try a bit more aggressive option of removing this fork, creating a new fork and submitting a new PR.

@dmwright526
Copy link
Copy Markdown
Collaborator Author

I think I see what is potentially happening... The change to "make_solo_mosaic.c" is happening when I merge the upstream develop, but for some reason it thinks there is a commit associated with this. In your instructions, it includes a flag "--no-commit", but that flag/option does not work on my end ("unknown option: --no-commit"). Is there another way to do this merge without tracking the file change?

@GeorgeGayno-NOAA
Copy link
Copy Markdown
Collaborator

I think I see what is potentially happening... The change to "make_solo_mosaic.c" is happening when I merge the upstream develop, but for some reason it thinks there is a commit associated with this. In your instructions, it includes a flag "--no-commit", but that flag/option does not work on my end ("unknown option: --no-commit"). Is there another way to do this merge without tracking the file change?

Try the merge without the --no-commit. The end result should be your fork's version of 'develop' should match the 'develop' in the authoritative repository. Then, merge from your fork's develop to your branch.

@dmwright526
Copy link
Copy Markdown
Collaborator Author

I tried that but it is still tacking on Gerard's commit... If you want to explore more about why this is happening, I am happy to keep this conversation going, but I don't want to waste your time on a minor issue. If you are fine with it, I can go ahead and close this PR, create a new fork of the repository to give a clean start, then submit a new PR with the correct develop branch. Luckily the code is modular enough that it shouldn't take too long to do.

@GeorgeGayno-NOAA
Copy link
Copy Markdown
Collaborator

I tried that but it is still tacking on Gerard's commit... If you want to explore more about why this is happening, I am happy to keep this conversation going, but I don't want to waste your time on a minor issue. If you are fine with it, I can go ahead and close this PR, create a new fork of the repository to give a clean start, then submit a new PR with the correct develop branch. Luckily the code is modular enough that it shouldn't take too long to do.

You should not have to delete your fork. Just delete your branch, create a new branch from the head of develop, then issue another pull request. Sorry this has been so complicated.

@GeorgeGayno-NOAA
Copy link
Copy Markdown
Collaborator

Looks like your fork's develop is out of sync with the authoritative repository. Go ahead and recreate your fork.

@dmwright526
Copy link
Copy Markdown
Collaborator Author

Ok, second PR has been created. No need to apologize. I am sorry this has been such a problem for you!

@GeorgeGayno-NOAA
Copy link
Copy Markdown
Collaborator

Closing this PR. Replaced by PR #158.

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.

5 participants