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

all zero values in atmImp_Faxa_lwnet over ocean for cpld_control_p7 test #914

Closed
uturuncoglu opened this issue Nov 16, 2021 · 40 comments · Fixed by NOAA-EMC/fv3atm#434 or #921
Closed
Labels
bug Something isn't working

Comments

@uturuncoglu
Copy link
Collaborator

Description

atmImp_Faxa_lwnet variable in the mediator history output has all zero over the ocean. This is probably passed to the other model components such as MOM6 that could cause an issue.

To Reproduce:

What compilers/machines are you seeing this with? Intel+MPT on Cheyenne
Give explicit steps to reproduce the behavior.

  1. checkout ufs-wearher model (head of develop)
  2. add mediator history phase to the run sequence in nems.configure (MED med_phases_history_write)
  3. run model
  4. check atmImp_Faxa_lwnet in the mediator history file.

Output

Screen Shot 2021-11-16 at 9 55 36 AM

@uturuncoglu uturuncoglu added the bug Something isn't working label Nov 16, 2021
@DeniseWorthen
Copy link
Collaborator

This can also be seen in the output files for MOM6, without using the mediator history files. In the file ocn_2021_03_23_03.nc, which is the last MOM6 output file for the cpld_control_p7 test, the field lw shows constant values of 0.0 except for 6 non-zero points on the coast of Baffin Is.

@uturuncoglu
Copy link
Collaborator Author

@DeniseWorthen it is weird that this is not caught with the RTs. I was expecting to see difference in the ocean fields and RT needs to be failed. Maybe we need to think about it.

@DeniseWorthen
Copy link
Collaborator

I confirmed that the issue arose at commit b87cdaa. That commit changed baselines. @SMoorthi-emc could you please take a look?

@uturuncoglu
Copy link
Collaborator Author

@DeniseWorthen That explains why this is not caught by the RTs but again I think that there is a gap in the current RT system that needs to be considered for the future improvement of the testing strategy. Maybe a set of graphical output could be generated along with the RTs. As I know this is done with the CESM but probably Mariana has better idea. At least for the single platform, a diagnostic tool can be used to create set of plots of difference from the previous baseline, publish in a web site that all the developers/users could look at visually to see any possible issue. It would be also helpful for the users to see the actual baseline changes through the output.

@DeniseWorthen
Copy link
Collaborator

@uturuncoglu I agree. A system like that would be very useful.

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Nov 16, 2021 via email

@uturuncoglu
Copy link
Collaborator Author

@DeniseWorthen I am not sure but it would be nice to create another issue for the discussion about RT regression system. We could leverage form NCAR's experience. Of course they are looking for different properties such as long term drift etc. but it would be still helpful. Maybe we could have a meeting just for focusing this. I think there are bunch of existing data analysis script in NOAA side that could be used for this purpose. It is just a mater of collecting them in a separate GitHub repository and integrate with the existing RT system.

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Nov 16, 2021 via email

@junwang-noaa
Copy link
Collaborator

@uturuncoglu That is a good idea! Looking forward to seeing what metrics are used for the coupled system and how the graphics are integrated in the test systems in CESM.

@DeniseWorthen
Copy link
Collaborator

I ran commits 5a20ea4 (prior to yours) and then b87cdaa (yours). I looked at the coupler history files for the two, and saw the impact only after the emissivity commit.

@uturuncoglu
Copy link
Collaborator Author

@junwang-noaa maybe we could use one of the Friday's call for this purpose. It would be very useful.

@junwang-noaa
Copy link
Collaborator

Great, let's include Rocky to see if he can set up a meeting. @rsdunlapiv

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Nov 16, 2021 via email

@DeniseWorthen
Copy link
Collaborator

You can set dbug_flag = 6 in nems.configure; doing so will turn on the state_diagnose function for FV3.

That confirms that in the export state, the values are zero:

20211116 112729.170 INFO             PET100 (FV3: state_diagnose) :ES:mean_net_lw_flx    0.000000      0.000000      0.000000

compared to previously

20211116 112055.870 INFO             PET100 (FV3: state_diagnose) :ES:mean_net_lw_flx   -90.20896     -28.57923     -21555.13

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Nov 16, 2021 via email

@DeniseWorthen
Copy link
Collaborator

Yes, but the two state_diagnose lines I wrote out were for your commit and the one immediately prior.

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Nov 16, 2021 via email

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Nov 16, 2021 via email

@uturuncoglu
Copy link
Collaborator Author

I wonder if any update about this issue? It would be hard for me to compare CMEPS calculated fluxes without a fix. Please also let me know if there is a version of the ufs-weather-model that I could use and has no this issue.

@junwang-noaa
Copy link
Collaborator

@uturuncoglu You can use revision 5a20ea4, which does not have this issue.

@uturuncoglu
Copy link
Collaborator Author

@junwang-noaa Okay. That is great. Thanks.

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Nov 18, 2021 via email

@SMoorthi-emc
Copy link
Contributor

Yes, it works!
Net_lw_into_ocn

@AvichalMehra-NOAA
Copy link

AvichalMehra-NOAA commented Nov 18, 2021 via email

@DeniseWorthen
Copy link
Collaborator

DeniseWorthen commented Nov 18, 2021

@SMoorthi-emc You said earlier in the thread that the ATM sfc flux fields looked fine. But the figure also looks like a sfc flux field? So was this field zero earlier?

@SMoorthi-emc
Copy link
Contributor

I found the bug and fixed it in my branch yesterday and created an issue.
I also attached an updated figure in another thread yesterday, but no one seems to have seen it.
Fix is a "one' line change in GFS_Typedef. The error occurred because of initialization of interstitial variables.

@SMoorthi-emc
Copy link
Contributor

Sorry, this is the thread - I see that Avichal and Denise have seen it. The above figure is from the MOM6 output (I am not sure if it is on a tri-polar grid). I also don't know how to template this output in grads. Note that this is net long wave - initially I was looking at the downward long wave in the atm files, which looked fine.

@DeniseWorthen
Copy link
Collaborator

This is the tripole grid if it is MOM6 output. I can also tell from the blank space around 120W, north of 70N. That is actually land in the tripole grid, so it shows up as no values.

Thanks for the explanation of why earlier you said the sfc flux files looked fine.

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Nov 19, 2021 via email

@junwang-noaa
Copy link
Collaborator

@SMoorthi-emc Thanks for fixing this issue! Since the fix is in GFS_Typedef, are you going to create an FV3 PR associated with the fv3 issue you created?

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Nov 19, 2021 via email

@junwang-noaa
Copy link
Collaborator

Thanks, we will combine this change with the next PR that needs new baseline.

@DeniseWorthen
Copy link
Collaborator

I also checked the analogous file which was shown in the original issue and verified that the field is now non-zero

Screen Shot 2021-11-19 at 9 00 19 AM

:

@climbfuji
Copy link
Collaborator

@SMoorthi-emc (@DeniseWorthen @junwang-noaa) I want to double check that the values are not only non-zero, but also correct. I noted you said that you removed a singe line

Interstitial%semis_water     = clear_val

Where? In GFS_typedefs.F90 in the interstitial_phys_reset routine? Note that the GFS_interstitial_type is not a persistent variable and the resetting is done on purpose. The variable gets reused for every block that an MPI task owns and does not contain valid data at the end of the physics time step. Everything that is in the GFS_interstitial_type is transient and only valid while within the physics or radiation group of the SDF for the current block number.

If you need to back this data out directly, i.e. without copying it into another array within the physics group, then semis_water needs to be moved from the GFS_interstitial_type to another DDT.

I hope my explanation makes sense ...

@SMoorthi-emc
Copy link
Contributor

Dom, you are right I was removing from interstitial reset, because it was zeroing it out.. This is the result of complex CCPP implementation.
I could have added sfcemis_water to Sfcprop just like I did for ice, but water emissivity is just a constant defined in surface radiation.f and there is no point adding it to the restart fle. There are alternate ways to handle this, but the easiest for now is just to remove this line.

@SMoorthi-emc
Copy link
Contributor

If I forgot to mention, CCPP is not the easiest code to work with.

@DeniseWorthen
Copy link
Collaborator

I checked the same field at hash 5a20ea4 (one prior to the emissivity commit) and the above figure is nearly identical. So, presuming that the lwnet was correct before, the fix seems to be correct.

@SMoorthi-emc
Copy link
Contributor

SMoorthi-emc commented Nov 19, 2021 via email

@climbfuji
Copy link
Collaborator

Thanks for checking.

Thank you both, good to know that there is an easy fix for it.

@DeniseWorthen
Copy link
Collaborator

@uturuncoglu This will be fixed when we merge PR #921 on Monday morning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
6 participants