search

NCAR / DART

Data Assimilation Research Testbed
https://dart.ucar.edu/
Apache License 2.0
184 stars 139 forks source link

Airs converter docs #639

Closed hkershaw-brown closed 3 months ago

hkershaw-brown commented 5 months ago

Description:

The AIRS and AMSU-S had some out of date build suggestions for HDF EOS, and some scripts that reference the HPSS (decommissioned in 2021) DART requires a patch to the HDFEOS library to successfully run the converters.

This pull request is to update the documentation for building the converters using the patched library, updated mkmf.templates and pointing people to the dart-built hdfeos libraries on Derecho.

I've removed the unnecessary entries from obs_def_rttov_nml in the input.nml. Only the two options are needed for the converter. rttov12 and rttov13 have different namelist options, so running rttov13 with the whole rttov12 namelist causes an error on reading the obs_def_rttov_nml namelist

L1_AMSUA_to_netcdf.f90 does not work. I've removed references to it. I think the code should be removed. Edit: I have removed it. https://github.com/NCAR/DART/blob/77bb8c2d64ffb5a86ded071520476b6dc4dccc4c/observations/obs_converters/AIRS/L1_AMSUA_to_netcdf.f90#L60-L62

Fixes issue

Fixes #590 Fixes #534

Types of changes

Documentation changes needed?

Tests

Please describe any tests you ran to verify your changes.

Checklist for merging

Checklist for release

Testing Datasets

hkershaw-brown commented 5 months ago

Todo: add fix for https://github.com/NCAR/DART/issues/609 to this pull request. https://github.com/NCAR/DART/tree/airs_fix

hkershaw-brown commented 5 months ago

Note to reviewers, the documentation build for this pull request is available here: https://dart-documentation--639.org.readthedocs.build/en/639/

hkershaw-brown commented 4 months ago

@braczka can you take a look at this pull request and see if it makes sense for a user. I've pulled out all the out-of-date hdfeos build docs, and points people to the hdfeos libraries available on Derecho.

Cheers, Helen

braczka commented 4 months ago

@hkershaw-brown Looking through this a bit today -- for reference could you provide the AMSU/AIRS data file(s) you used for obs converter testing? I could download them myself, but the link at least for AMSU is broken, and want to make sure I am looking at the correct data files.

hkershaw-brown commented 4 months ago

Hi Brett, this is a good review comment. We need to point to where the data is, this is the broken link right? "Getting the data currently means putting in a start/stop time at this web page"

Here is some data Nick ws using (original reporter of the hdfeos library-dart bug): /glade/derecho/scratch/nickp/dart_obs/la/AIRS/202201/20220101

hkershaw-brown commented 4 months ago

https://disc.gsfc.nasa.gov/ ?

braczka commented 4 months ago

Thanks Helen -- just some clarification on the review -- did you want me to stick with the software side i.e. make sure I can build and use the converters without any issues only. The existing science/data documentation (ie. discussion of satellites, L1-L3 products) that we inherited from existing documentation could be streamlined and updated, but that would not necessarily have any bearing on the software functionality.

hkershaw-brown commented 4 months ago

Hi Brett, definitely the documentation, that is what this pull request is about. https://dart-documentation--639.org.readthedocs.build/en/639/observations/obs_converters/AIRS/README.html

hkershaw-brown commented 4 months ago

@hkershaw-brown I committed a bunch of docs changes already to this PR. Sorry -- didn't mean to be so heavy-handed. I actually meant to request permission before pushing those commits. I think you will be OK with them, as they are primarily related to scientific documentation only, not software. I am having trouble with hdf4 to netcdf conversion step for the convert_amsu_L1 converter. See my comment there -- otherwise things look good.

@braczka this is awesome stuff, don't worry about being heavy handed. I'll take a look at h4tonccf_nc4 - see I can get it built on Derecho.

braczka commented 3 months ago

I have completed my recommendations with commit f8ae543. The h4tonccf_nc4 converter works using @hkershaw-brown instructions above. I included doc edits to account for this, and additional metadata description for AMSU-A converter.

hkershaw-brown commented 3 months ago

@mjs2369 whenever you and Kevin think this is good to go, this can be bundled as a release with:

658 #638

kdraeder commented 3 months ago

Hi Brett, definitely the documentation, that is what this pull request is about. https://dart-documentation--639.org.readthedocs.build/en/639/observations/obs_converters/AIRS/README.html

Does this link lead to up-to-date docs? Or should I build my own from this branch(?)?

braczka commented 3 months ago

Hi Brett, definitely the documentation, that is what this pull request is about. https://dart-documentation--639.org.readthedocs.build/en/639/observations/obs_converters/AIRS/README.html

Does this link lead to up-to-date docs? Or should I build my own from this branch(?)?

Yes -- that link reflects the up-to-date docs. Alternatively, you can also click on 'Show all checks' and then the 'Details' link associated with the latest docs build to access the up-to-date docs.

kdraeder commented 3 months ago

Generally, it looks good. Here are a few minor suggestions.

I checked the links. I did not follow the instructions to set up DART+RTTOV, since Brett did that. I'll do that if it would be useful.

Consider making obs_def_rttov13_mod.f90 a link, or adding observations/forward_operators.

Could simplify: However, a particular model may not have all of the variables necessary for these functions depending on the model and model setup. -> However, a particular model and setup may not have all of the variables necessary for these functions.

Setting UP

  1. Consider adding a reference to a mkmf.template that uses RTTOV.

typo (formatting issue?): models//work

braczka commented 3 months ago

Generally, it looks good. Here are a few minor suggestions.

I checked the links. I did not follow the instructions to set up DART+RTTOV, since Brett did that. I'll do that if it would be useful.

Consider making obs_def_rttov13_mod.f90 a link, or adding observations/forward_operators.

Could simplify: However, a particular model may not have all of the variables necessary for these functions depending on the model and model setup. -> However, a particular model and setup may not have all of the variables necessary for these functions.

Setting UP 2. Consider adding a reference to a mkmf.template that uses RTTOV.

typo (formatting issue?): models//work

@kdraeder There might be some confusion. It looks like you reviewed the DART-RTTOV documentation, but the documentation associated with this PR is for the AIRS and AMSU-A obs converter. Which also leads to links for the AIRS and AMSU-A documentation, which were also revised as part of this PR. Given the dependence of the AMSU-A converter on RTTOV we could also make that part of this PR, but not sure that was the original intent.

kdraeder commented 3 months ago

Generally, it looks good. Here are a few minor suggestions. I checked the links. I did not follow the instructions to set up DART+RTTOV, since Brett did that. I'll do that if it would be useful. Consider making obs_def_rttov13_mod.f90 a link, or adding observations/forward_operators. Could simplify: However, a particular model may not have all of the variables necessary for these functions depending on the model and model setup. -> However, a particular model and setup may not have all of the variables necessary for these functions. Setting UP 2. Consider adding a reference to a mkmf.template that uses RTTOV. typo (formatting issue?): models//work

@kdraeder There might be some confusion. It looks like you reviewed the DART-RTTOV documentation, but the documentation associated with this PR is for the AIRS and AMSU-A obs converter. Which also leads to links for the AIRS and AMSU-A documentation, which were also revised as part of this PR. Given the dependence of the AMSU-A converter on RTTOV we could also make that part of this PR, but not sure that was the original intent.

Sorry, I confused myself by knowing just the wrong amount about the connection between AIRS and RTTOV. My comments about RTTOV should not become part of this PR. I'll review the right files (below).

braczka commented 3 months ago

@mjs2369 Do you know why the doc build failed for a few of the latest syntax fixes?

mjs2369 commented 3 months ago

@mjs2369 Do you know why the doc build failed for a few of the latest syntax fixes?

@braczka

It's because I triggered the builds all at once by pushing the commits in succession in 10 seconds or less. So the doc builds for the first three commits didn't have time to finish building and were cancelled when the next one was triggered.

If you look at the details of the checks for the docs, it shows the error message that says 'The user has cancelled this build.' So anytime you push multiple commits within the time it takes for the docs to finish building (usually 1-2 mins), this will happen and is nothing worry about. That is also why the last commit in that series had all checks pass successfully.

You can always see the details of the actions checks by clicking on the little checkmark or red x next to the commit and then clicking Details on the check you're interested in. In this case, doing so for the failed doc builds should take you to a page like this: https://readthedocs.org/projects/dart-documentation/builds/23879246/

braczka commented 3 months ago

@mjs2369 Do you know why the doc build failed for a few of the latest syntax fixes?

@braczka

It's because I triggered the builds all at once by pushing the commits in succession in 10 seconds or less. So the doc builds for the first three commits didn't have time to finish building and were cancelled when the next one was triggered.

If you look at the details of the checks for the docs, it shows the error message that says 'The user has cancelled this build.' So anytime you push multiple commits within the time it takes for the docs to finish building (usually 1-2 mins), this will happen and is nothing worry about. That is also why the last commit in that series had all checks pass successfully.

You can always see the details of the actions checks by clicking on the little checkmark or red x next to the commit and then clicking Details on the check you're interested in. In this case, doing so for the failed doc builds should take you to a page like this: https://readthedocs.org/projects/dart-documentation/builds/23879246/

OK -- sounds like its pretty normal, although when I do check out the details all it says is the user cancelled the build, which is misleading. The only reason I ask is that @kdraeder is reviewing right now, and could be a little confusing in that the latest doc build won't include those syntax changes (but they will be included in the file changes)

mjs2369 commented 3 months ago

OK -- sounds like its pretty normal, although when I do check out the details all it says is the user cancelled the build, which is misleading. The only reason I ask is that @kdraeder is reviewing right now, and could be a little confusing in that the latest doc build won't include those syntax changes (but they will be included in the file changes)

I can understand how it might be a little misleading since I didn't directly 'cancel' the builds. For ReadTheDocs, pushing another commit before the build finishes effectively cancels it.

The latest doc build (which finished successfully) will include the syntax changes from all the previous commits as well. So there shouldn't be any inconsistencies between the file changes and the latest build of the documentation. I checked this earlier to make sure everything formatted correctly on the website and all the changes were present.