-
Notifications
You must be signed in to change notification settings - Fork 14
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
add export of cpl_scalars #8
add export of cpl_scalars #8
Conversation
* export 2dsize of tile and number of tiles for use by CMEPS mediatory history files
@uturuncoglu This is working fine for UFS. I am able to get the land history files on 6 tiles. I have a run here
The file |
@DeniseWorthen I am not sure there is an issue or not but please see following plots. The first one is I am not sure we have same issue with original implementation or not. It seems other variables have same issue but since the units are different in model history and mediator history, it is hard to tell. |
@DeniseWorthen It would be also nice to run |
Hm....the One test I didn't do is to check using the current method of setting the tilesize via config. I think you had used that previous. The new history files should be equivalent. |
@DeniseWorthen Yes. The land output also includes the forcing fields coming from atmosphere. So, for example |
@DeniseWorthen Yes, if I remember correctly, I was using config options also for land but it is hard to remember exactly. |
@uturuncoglu I made a couple of checks. First, I wrote the mediator history files on the mesh and decomposed them into the tiles using NCL. Those fields match the import/export fields I get if I use the original method of setting a config variable
This original method only worked for component history files, but even when the fields are split up into two files (one for atm history and one for lnd history), the fields are identical to the original mesh fields. Then I checked that the fields using the new Second, I looked at time=25200 on tile 1 and the zvfun in the "out" file compared to the lndImp_Sl_zvfun and the atmexp_Sl_zfun in the mediator history. So the lndimp looks the same as the lnd out file, but the atmexp is different. I don't know whether that is expected. I also checked the DSWSFC field from the "out" and the LNDEXP_FAXA_SWDN and ATMIMP_FAXA_SWDN fields and those all look consistent. |
@DeniseWorthen Thanks for checking. Yes, it seems that there is an issue in atm export. I think that |
@DeniseWorthen @barlage I was also looking to the debug mode issue. I put required checks to the place that we have issue and still failing. In think I know why, we are using There are lots of differences and I think there are also land-sea mask difference between those files. And it leads to issue. I could modify the code and make it consistent by using also src file to construct land-sea mask. Just to be sure, I also need to check datm coupled configuration since that uses custom initial condition. @barlage let me know what you think? @DeniseWorthen I think once I fix the issue, I think this could be part of your PR since you have some modification in the land model too. Of course we also need to solve the issue in |
@DeniseWorthen @barlage I think this is also related to the ufs-community/ufs-weather-model#1423 |
@DeniseWorthen @barlage Okay. I fixed the issue with vegetation fraction but now getting error from soil type. This is also read from
but I think surface file does not have all of those variables and we will end up with inconsistencies on those variables in terms of land-sea mask and actual values like I showed for zvfun. |
I did run the develop branch and use NCL to write out the fields on the tiles. I see the same thing---the lndImp and the atmExp of Sl_zfun are not the same, and I agree they should be. But, I also don't understand what the field "land fraction" fields mean. The lfrac in the atm looks the same as the lfrac in the land, but I don't understand how lfrac is zero on land. It must mean something different than I think: |
@barlage @DeniseWorthen I made some changes in the code to solve the issue under debug mode which is triggered by inconsistencies in the input data. So, following is the initial attempt to solve it, https://github.com/NOAA-EMC/noahmp/compare/develop...NOAA-EMC:noahmp:hotfix/debug_mode?expand=1 This is working and passing the point that Denise had issue but failing with another error like following,
This is basically caused by the soil color data. If you look at my changes closely, I am still getting soil color data from static file rather than sfc file and this causes issue since those are not consistent. I am not sure how this can be solved. Two possible options are (1) create new initial condition (sfc file) that has also other variables line soil color (I still don't sure what the status soil color does not match with sfc file in terms of masking) or (2) update soil color static file. @barlage let me know what you think? |
@DeniseWorthen I think this is not related with the land fraction field and I think we have issues with other fields. Maybe some recent changes in the CMEPS side. The baseline created after land PR is,
and current baseline is,
The BTW, if I check the FV3 output from your run |
@DeniseWorthen Maybe there is something wrong with land frac. So, that leads issue when data is transferred from land to atmosphere. Anyway, I am trying to run in my side to test some ideas. |
@uturuncoglu Maybe Mike understands better, but I know George recently committed a new "v2" surface file test. I don't know if those files are more appropriate to use (the old land mask mis-match issue?). I feel like I opened a can of worms but I do think the cpl_scalars are working correctly (except for ESCOMP/CMEPS#436 (comment)). Let me know what I can do to help. |
@DeniseWorthen Yes, file inconsistencies are the main issue in here. I am glad that we have issue and It force us to fix it. We have weekly tag-up with @barlage today. So, we could discuss it. I agree it seems that I/O part seems working. I am still working on land fraction issue, I might solve it today. |
@DeniseWorthen I investigate it little bit further. Here is my finding, the following part of the code in CMEPS basically assigns the lfrac, https://github.com/NOAA-EMC/CMEPS/blob/624920ddbd819c76ec37591c24e872308201810e/mediator/med_fraction_mod.F90#L257. I checked both |
@uturuncoglu good digging. I wonder what it means when it says as a comment "this may be overwritten later" (in the fraction_mod). |
@DeniseWorthen Yes. In the fraction file, the mediator trying to map land fraction coming from atmosphere as land fraction. I put more debug code to cmeps to find out this is the issue or not. If so, we might need to put some specialization in here. I am not sure configuration under CESM. Maybe there is some implicit assumption over there. I think FV3 does not export land fraction but need to check. |
@DeniseWorthen @uturuncoglu I did a little digging into the sfc_data and fix files being used in the RT. What I mean below by "not consistent" is that there are not valid land characteristics over all grids where land_frac>0. From what I can tell:
Note these are consistent for non-fractional grids.
|
@barlage Thanks for checking. Since the test uses |
@DeniseWorthen Following part of code changes the land fraction. https://github.com/NOAA-EMC/CMEPS/blob/624920ddbd819c76ec37591c24e872308201810e/mediator/med_fraction_mod.F90#L454. It seems that 'lfrac' in FBFrac(compatm) is set by https://github.com/NOAA-EMC/CMEPS/blob/624920ddbd819c76ec37591c24e872308201810e/mediator/med_fraction_mod.F90#L390 like following
using ocean fraction but since we don't have ocean component and using |
@DeniseWorthen Okay. The part that sets the land fraction using ocean is just used when either ocean and ice model present. I think we need to add extra protection to 822L to prevent getting land fraction from atmospheric model component. |
Sorry line 461 in As you can see, we have lots of zeros over land which is not realistic and created issue. I think I our case, we have to use land provided fraction rather than the one interpolated from atmosphere which is not correct for UFS case. |
@DeniseWorthen Maybe the mapping in https://github.com/NOAA-EMC/CMEPS/blob/624920ddbd819c76ec37591c24e872308201810e/mediator/med_fraction_mod.F90#L431 is not working correctly in out case since it is trying to map land frac from land component to atm. |
@DeniseWorthen Okay. I traced the issue more. It seems that atmospheric model mask shows the same issue. Mediator log has following,
So, since destination mask (fv3 model) is corrupted the interpolation has also issue. I checked the fv3 configuration and it seems it sets the mask in
also |
@uturuncoglu What I still don't understand is what land fraction should be on land, away from a coast. Near a coast, "land frac" in FV3 has a meaning relating to the mapped ocean mask. That is the 1-land_frac in the below figure (it is called land_frac, but it is really ocean_frac) used in the setting of the lsmask variable. Does the land model have a concept of "fractional land", so that a grid cell in the middle of the Sahara would be something other than land_frac=1? Your satellite view figure above agrees w/ the variable |
So what really have we changed here? We're not changing the lsmask in atmos_model, but you've fixed a couple of bugs in noahmp and we've changed the mapping masks. But you were testing all of that earlier today, when you were getting bad VTK files? Is the only difference now that we're using v2 surface files in addition to the other changes? I wanted to see if the fix is really coming from v2 surface files. I tried to run the same executable (hotfix + cmeps mapping) w/ the old surface files and I still get a set fault
|
@DeniseWorthen The fix does not fix the issue with the old surface files since we are still getting soil color from fixed file not from sfc file. In v2 case, those files are consistent (soil color too) and it is working. I did not check that the v2 sfc files has soil color in it. If so, I could get that one too. Anyway, the code crashes since old files are not consistent in terms of soil color. |
@uturuncoglu I don't think it necessary to make the current code work w/ the old input, I'm just wondering what really "fixed" the land mask issue? We definitely need to commit a debug atm-land test. |
@DeniseWorthen Okay. Good news. It seems that optimized one also working fine. We have no land fraction issue anymore. Please double check in your end too if you could find time. Anyway, let me work on little bit more in noahmp fix branch and clean it and if soil color is available in sfc file I could get it from there too. I'll update you about it. In the mean time you could get the mask change to your CMEPS PR. I also need to run other cases to be sure that sbs and datm coupled ones are fine. I am not expecting any issue but just to be in the safe side. I think with this changes, we will have answer change in the land related tests. I am also plaining to add land debug test. Do you want me to crate ufs-weather-model branch that you could get it to your PR? |
Good morning @uturuncoglu. I was hoping to bring in the cpl_scalars work to UFS in a single PR w/ no baseline changes (except for the single history file which is compared and which now is on tiles). So I'd like to do the following if it works for you.
Let me know if this all works for you. |
@DeniseWorthen yes, that works. Then, I could create fix PR. BTW, It seems everything is fine except 2m specific humidity. We have some small values around coastline. I am not sure this is v2 data files issue or not. Are you seeing same issue in your end? |
Let me check. I didn't specifically look at q2m. |
@DeniseWorthen Thanks for checking. Did you also look at t2m? That has no similar issue. Anyway, there might be a bug in CCPP Level too. So, I'll check it. Anyway, please ad any piece to your PR and I could start working from that point. Thanks for all your help. |
I see the T2m temperature in the sfc file showing the same 0.0 values along the coast as the specific humidity. I didn't want to push the two fixes to my ESCOMP CMEPS PR in case there is still an issue in CMEPS mapping. I don't think there is, because the lnd import and atm exports look OK. But I wanted to check w/ you first. |
@DeniseWorthen Okay it is good to know we have issue in t2m. In my case, I could not see it. How did you do it? BTW, I think CMEPS mask change for lnd->atm is fine. |
@DeniseWorthen In my case, if I check sfc file the |
@DeniseWorthen Okay. Got it. |
@DeniseWorthen BTW, i am plaining to open new issue in UFS Weather Model side to track the issue specific to lnd->atm coupling and also inconsistencies about the input files. I could use it also in the fix PR that will come soon. |
@uturuncoglu I'm guessing at how to match mediator history variables and surface file variables. But this is T2m from sfc file and sa_ta from the mediator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks fine. Thanks for adding cpl_scalars
support. The GitHub action is also passing for DATM+LND configuration. @barlage please let me know if you want to review this PR.
@DeniseWorthen @barlage JFYI, I create a new issue under UFS Weather Model to resolve the issues about inconsistent input and also debug mode run. ufs-community/ufs-weather-model#2189 |
@uturuncoglu Great. I've run the |
@DeniseWorthen Yes. Thanks for confirming. If I remember correctly, datm coupled configurations and sbs were fine with this change in my tests. Anyway, I am working on it. |
@DeniseWorthen I created a draft PR in here ufs-community/ufs-weather-model#2191. This will go after yours since depends on CMEPS changes. |
@uturuncoglu Thanks for your followup work on this. Let me know if I can help to test. I'll create the final sync-to-emc CMEPS PR once you merge (assuming no issues). |
Maybe @uturuncoglu can merge also? |
@DeniseWorthen @zach1221 Okay. I merge it. |
Export 2d-size of tile and number of tiles for use by CMEPS mediator history files.
See additional information in UFS Issue ufs-community/ufs-weather-model#2171