-
Notifications
You must be signed in to change notification settings - Fork 13
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 validated aircraft yamls #128
Add validated aircraft yamls #128
Conversation
I think it is a good way to have yaml for each obtyp at the early stage of RDASApp development. |
@ShunLiu-NOAA, thanks for that feedback. As @guoqing-noaa suggested, I was trying to test these yamls more extensively in MPAS-JEDI. I had previously only tested airTemperature which I can confirm works after removing both ob inflation parts, changing the ob input file, and removing the GOMsaver (I think that was all the changes). I'm not able to get the wind yamls to work due to
I also tried |
I see |
Yes, that is correct. the example is here: also, test3dvar.yaml |
Thanks @HuiLiu-NOAA!! |
Hi @guoqing-noaa and @spanNOAA, Do you mean uReconstructMeridional and uReconstructZonal are not in the restart.nc? I think that is why I can't get the wind assimilation to work in MPAS-JEDI testing. Does this mean we need to regenerate the test case or do we just need to have those extra files to have access to that info in addition to the updated submodule? |
@delippi I think 10m winds are diagnostic variables. They are not uReconstructMeridional and uReconstructZonal which should be 3D winds |
@guoqing-noaa, I'm getting a warning that uReconstructZonal/Meridional are not in input file. Can that variable be written to the restart.nc file? I see it appears in the analysis.nc file. If we eventually use 10m winds as background for surface type obs that is fine, but we still need to have these 3D winds for non-surface type obs. In my validation I've been mimicking what GSI does which is to use the 3D winds for surface obs. But also, u10 already appears to be in the restart.nc file. |
…yamls to work in mpas-jedi configuration. Changes include: placeholder for obs_filname, commenting out ObsErrorFactorConventional since PreQC is the expected QCflag, updated the ObsErrorFactorPressureCheck to use surface_altitude instead of surface_geometric_height, and added some time filtering. Finally, updated gen_yaml.sh to create super yaml more easily since now it is required to specify the obs_filename.
@ShunLiu-NOAA @guoqing-noaa, everything works in MPAS-JEDI except for the humidity yamls which is because MPAS-JEDI is having trouble creating the saturation_specific_humidity, in my case. I tested this with the new mpasout backgrounds. Perhaps I'm just missing something in the configuration of the test case? I've added them here anyway, but I do need to figure out how to make those work, but they should be really close. There were a few strange things I've found that I need to dig into a little. I ran each yaml independently and then checked the increments. For a few cases (including uv_288, which is a big one) I got 0 increments. I tracked that down to the ObsErrorFactorPressureCheck (oberr inflation) and turned it off and the increments were fine. I haven't been able to track down the cause of this, but it over inflates the obs to really large value and so none of the obs gets used. This also happened for uv_235. I think overall these yamls "work" for MPAS-JEDI cycling purposes. There is obviously still much more work to do to make sure they are good science wise. We also need to update the staged ioda_mesonet.nc file since that one seemed to have been made with an old yaml. |
@delippi Thank you for the update. So after we update ioda_mesonet.nc, we can test the changes in this PR and merge your changes into develop. |
@ShunLiu-NOAA, I did test the mesonet with the updated ioda_mesonet.nc file. I just need to put that into the staged locations. But also, I don't have a domain check in these yamls yet. I also had to use my offline domain check run at 10% hull shrink to get things to work. There were obs that appear to be inside the domain, but still don't work for some reason. They are all obs somewhat close to the boundary. Should I also update both the aircraft and mesonet ioda files with the offline domain check versions? |
@delippi Thank you. Please update both the aircraft and mesonet ioda files with the offline domain check versions. |
… filters, updated threshold and absolute threshold and timOffset min/max values to match the values in RRFS_A, reduced inflation factor from 8,4 to zero for all cases since there seems to be some issue with that part of ObsErrorFactorPressureCheck in mpas-jedi, turned off completely ObsErrorFactorPressureCheck error inflation for specificHumidity obs, increased the the temporal thinning min spacing to 30M. Results now seem reasonable.
…ted the uv obtype configs to specify which obtype (e.g., 233) for each QC filter so there is no ambiguity.
@ShunLiu-NOAA, @guoqing-noaa is helping to update the aircraft and mesonet ioda files that I have generated. I've also just finished some more testing. I believe everything is in working order now. I performed an individual test for each obtype (16 in total here since there are 16 yamls) to check the impacts of each obtype on the analysis. I made some adjustments to get reasonable increments. I also performed a test where I concatenate all 16 yamls into a super yaml and make sure that is also working. I think these yamls are ready to go into a cycling mpas-jedi system when we have that going. Here is the result of that all aircraft+mesonet ob test instead of showing the 16 increment plots: |
Hi @guoqing-noaa, I was planning to update both of those files. The mesonet file was the only one regenerated. I used bufr2ioda converter to do that (so not from gsidiag). Then I used the offline domain check to filter out some obs that were giving trouble in the southern part of the domain for both ioda files. That might be related to issues with the case itself, but it is probably fine to update the obs anyway (see: #160 (comment)). |
I have each obtype as its own yaml. So I have an individual, fully working yaml for aircft_airTemperature_130, aircft_airTemperature_131, aircft_airTemperature_133, and so on. Basically the super yaml just a concatenation all of them together. It might not be the most efficient in terms of yaml length (its like 2800 lines for these 16 obtypes) but having each obtype in its own file that you can just append all together is a lot easier to manage IMO. And of course this is all temporary until we start using JCB. Here is an example: |
@guoqing-noaa, I'm now thinking those obs are just outside the domain and my plots are showing the domain edges incorrectly. I think I should just try to implement the domain check within the yamls and see if it fixes the issue. I think the mesonet ioda file will still need updated, but I will need to regenerate that without running the offline domain check. @ShunLiu-NOAA, let's wait to merge this PR until I've tested the domain check within the jedi yamls. |
@ShunLiu-NOAA & @guoqing-noaa, I think this PR is ready as is. I tested the use of the online domain check, but it doesn't work to remove the obs from memory so obs outside the domain will still cause jedi to fail during the ObsErrorFactorPressureCheck (ob error inflation). Therefore, I did not include that online domain check within the yamls. I currently just run the offline domain check before doing my tests. In terms of the updated obs files, I will be going through the current set of ioda obs files and make sure those were all generated using the correct yamls and also create ctests for them as well. I think that might all be best to go in another PR, however, I can provide a temporary mesonet ioda file that I was able to get to work for my tests sooner than that. I can also provide ob files that have run through the offline domain check too. |
Just to add a bit more details: The ObsErrorFactorPressureCheck (ob error inflation) in its current version seems out-of-date and not able to use the QC flag set by the online domain check, which is the root reason for Donnie;s issue. In future, this ObsErrorFactorPressureCheck should be updated to use the QC flags set by basic filters, and this issue would be resolved hopefully. |
Would it be better/safer to double check/test these yamls with the corrected obs ioda files? |
@HuiLiu-NOAA, thanks for adding some more context about the online domain check. Adding the ability for ObsErrorFactorPressureCheck to read those QC flags is one solution. I wasn't sure if there would be the option to drop the obs from memory eventually. There definitely needs to be some more work in this regard, but that should be done in another PR.
Good question. I don't see that as being necessary. I did test with a regenerated/corrected ioda_mesonet.nc file. And looking through each of the aircraft and the new mesonet ioda files everything looked as expected--I think those were generated with a new enough bufr2ioda yaml and likely won't change much if at all. The PR in which I will go through each ioda file will be mainly to document exactly which version of the bufr2ioda yamls and executable was used to generate those files and creating some ctests for them and correcting things as needed. I just took a look at the ioda_adpsfc.nc file that you were mentioning before. I think perhaps the main issue was with ObsError. I don't use the ObsError in the ioda files. In fact, I requested mesonet data not have ObsError at all in the ioda file. My preference is to not have them there at all and it should be up to the user to define the ObsErrors for each type within the JEDI yaml. |
Thanks @delippi for the clarifications! |
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.
Thanks for adding these @delippi. It will be nice to be able to incorporate all of these yamls into our ctests. And the updates to gen_yaml.sh
will help a lot on that front.
@guoqing-noaa Could you please review this PR again before we merge this PR to develop? Thanks. |
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.
LGTM. Thanks @delippi !
To confirm, we don't need to save any ioda files at this moment, right?
Correct, unless you want to save the temporary mesonet ioda that I created? Because the mesonet yamls here won't work with the current ioda file. I plan to work on all of these in another PR and check with Praveen which are the correct yamls to use. The aircraft yamls will only work after you run an offline domain check. I'm satisfied with them for the purposes of getting them ready for a cycling system. In the cycling system we just need to make sure that the mesonet ioda are generated correctly in that system and you run all the ioda obs through a prior offline domain check. When I regenerate the ioda for this test case I will have two versions of each ioda. One will be the raw ioda and the other will be the domain_checked version. |
@guoqing-noaa, I do have the ioda_aircraft.nc and ioda_mesonet.nc ready to go and double checked with these yamls. Why don't we go ahead and update those two files to have them available with this PR. |
… Changed instances of aircraft to aircft or mesonet to msonet for consistency. Made other consistency changes. Nothing changes results.
e636501
We had lots of great discussions at our ioda_bufr file meeting this Tuesday. I think it will be great if we can track those discussions/changes (by Donnie) in this repo (otherwise, we will do it in rrfs-workflow). To facilitate this, I propose we create a new directory |
@guoqing-noaa, I think that is an excellent idea. There is already a |
Is it possible to merge this PR @ShunLiu-NOAA, @guoqing-noaa, @delippi? I know we are still waiting on some of the observations to be staged, but merging this now would help quite a bit with my work in #178. |
@ShunLiu-NOAA & @guoqing-noaa, I think it is fine to merge now even without the new ob files staged yet. It won't be long until we get that all sorted out. If any developers need access to any preliminary set of obs I can provide those, if necessary. |
@hu5970 and @guoqing-noaa Let's merge this now and consider how to handle this in the workflow later. |
Sure. No problem. We can always update later as we learn more. Thanks! |
Added aircraft yamls, updated the gen_yaml script, renamed mesonet yamls, and a configuration to run with MPAS-JEDI.
In this PR, aircraft yamls using GSI-IODA and GSI-geovals in FV3-JEDI space will be merged to RDASApp. These obtype yamls, should be generic enough to work with either FV3- or MPAS-JEDI. The only thing might be that you cannot use the ObsErrorFactorPressureCheck (oberr inflation) in MPAS-JEDI since there seems to be some model-specific parts of the JEDI code expecting FV3.
I'm not sure if this is the best way to go about this. At the time of making this PR, I have each obtype in its own yaml like the following. In this way, there is a lot of repeated text, however, it makes it much easier to debug typos and create new yamls. If there is a better idea for how to manage this let me know. Although this is just a short term solution before we move to JCB.
The script (
gen_yaml.sh
) to concatenate all of these yamls has also been updated.Results associated with this PR: Issue #106