-
Notifications
You must be signed in to change notification settings - Fork 145
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
Remove offset on GOES obs converter #732
Conversation
Hi Brett, can you provide 2 GOES ABI observation files for testing since we also have this issue outstanding: #579 |
My work directory for testing the GOES obs converter (convert_goes_ABI_L1b) is located here: An example of a GOES radiance raw data file is located there : FYI -- my folder for testing the RTTOV channel offset bug is here: |
two would be great since the other bug is "bug: GOES converter does not handle multiple input files" |
Done |
If you build the converter with rttov13, then run, it errors out on the namelist.
|
Sorry -- the input.nml file got reverted to the default. The input.nml I used for the GOES conversion is here: |
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.
see default input.nml comment
Changes made to make input.nml compatible to run the converter. Should be ready for approval. |
ok I haven't actually looked at this pull request since trying to run it. I'll update to main and take a look. |
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.
Hi Brett,
A few things
There's a broken link in to the GEOS converter, typo $ vs ~
convert_goes_ABI_L1b is missing from DART/.gitignore
Some of the documentation on rttov database vs. rttov coeffiencts is not that clear (e.g. the database is a DART file)
The vertical location for radiance obs I think would benefit from mentioning the namelist option to set a vertical location.
ps. I'm waiting for a Derecho job to actually run the code.
Cheers,
Helen
HIMAWARI_9_AHI 31 9 56 ir rtcoef_himawari_9_ahi.dat | ||
|
||
The coefficent file (.dat) is included with the RTTOV installation and can be found at the | ||
path ``~{RTTOV_install}/rtcoef_rttov13/rttov9pred54L/rtcoef_himawari_9_ahi.dat``. This file |
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.
$path to the rttov install, rather than the home directory of ~rttov_install
path ``~{RTTOV_install}/rtcoef_rttov13/rttov9pred54L/rtcoef_himawari_9_ahi.dat``. This file | |
path ``${RTTOV_install}/rtcoef_rttov13/rttov9pred54L/rtcoef_himawari_9_ahi.dat``. This file |
288.370817896852 | ||
288.456378689407 |
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.
Looks like two observations copies in this example. Do you want two copies? I think for an obs_seq.out it would normally be obs & qc:
288.456378689407
0.000000000000000E+000
provides a 4 integer description (31/9/56/8) of the platform/satellite/sensor/channel | ||
combination specific to this satellite observation. For more information on this | ||
metadata refer to this GOES observation converter example here: | ||
:doc:`../observations/obs_converters/GOES/README.rst` |
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 link is not correct and so does not get built:
/Users/hkershaw/DART/pull_requests/pull_732/observations/forward_operators/obs_def_rttov_mod.rst:105: WARNING: unknown document: '../observations/obs_converters/GOES/README.rst'
The relative path is incorrect and you have .rst on the file
:doc:`../observations/obs_converters/GOES/README.rst` | |
:doc:`../../observations/obs_converters/GOES/README` |
or add an anchor to the GEOS/README.rst file and link to that anchor.
as undefined (VERTISUNDEF, integer = -2), however, this limits the ability to vertically | ||
localize the impact of the observation on the model state. | ||
|
||
The RTTOV specific metadata is located after the ``visir`` line. This includes the |
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 example if for visir observations, the microwave observations have
'mw ' instead of visir'
the file contains all wavelengths versus only IR wavelengths is **extremely important** because | ||
it will shift the value of the channel number. Recommended practice is to choose a coefficient file | ||
with all channels included. If, on the other hand, you subset your coefficent file to only include | ||
IR channels, you will also have to adjust your channel number in the obs_seq.out file. |
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.
For this, are you suggesting people edit the obs_seq.out file or run the converter with the correct channels? It is probably better to tell people to change the converter if they are using a subset of channels.
the vertical location undefined (i.e. VERTISUNDEF) within the ``obs_seq.out`` | ||
file. This approach, however, limits the application of vertical localization | ||
during the assimilation step. | ||
|
||
Alternatively, for some applications it may be appropriate to assign | ||
a vertical location to the radiance observation. This is an ongoing area | ||
of observation-space localization research, and is the standard | ||
workaround pioneered by Lili Lei and Jeff Whittaker. |
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.
Is this section "Specifying a vertical location" talking about the namelist option vloc_pres_hPa
?
I think it would be better to explain the vertical choice in terms of the namelist option for the GEOS converter.
The default is to have VERTISUNDEFINED. If you want to set a vertical location (science blah blah), set vloc_pres_hPa to the vertical location of the observation in hPa.
goes_channel = map%channel-6 | ||
! Defines the GOES ABI wavelength channels (band_id) | ||
! Channels range from 1-16 covering UV/VIS/NIR/IR spectrum | ||
goes_channel = map%channel |
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.
note in release notes, this will affect people using subsetted rttov files.
.. Important :: | ||
|
||
**It is imperative that the user confirms the satellite integer metadata matches the | ||
appropriate RTTOV coefficient (parameter) file. See next section for more information.** |
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.
It is a little hard to tell which file is which from the description.
radiance observation. The RTTOV database file (``rttov_sensor_db.csv``) refers to the coefficent | ||
file. For the ``HIMAWARI_9_AHI_RADIANCE`` observation type, for example, the following information |
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.
The rttov_sensor_db.csv a DART file, correct? I would be good to make that clear. At the moment it reads like the RTTOV database file is something you should be getting from RTTOV.
The namelist option:
rttov_sensor_db_file | character(len=512) | The location of the RTTOV sensor database. The format for the database is a comma-separated file. The columns of the database are the DART observation-kind, the platform/satellite/sensor ID, the observation type, the coefficient file, and a comma-separated list of RTTOV channels to use for this observation type. |
---|
looking at the code the channel list is optional:
! channel_list is an optional list of channels (can be zero-length, meaning all available channels should be used) |
there are no channels in the rttov_sensor_db.csv sensor file in the repo:
NOAA_1_VTPR1,1,1,7,ir,rtcoef_noaa_1_vtpr1.dat
NOAA_2_VTPR1,1,2,7,ir,rtcoef_noaa_2_vtpr1.dat
NOAA_3_VTPR1,1,3,7,ir,rtcoef_noaa_3_vtpr1.dat
...
are users supposed to edit rttov_sensor_db.csv or leave it as is?
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.
The rttov_sensor_db.csv a DART file, correct? I would be good to make that clear. At the moment it reads like the RTTOV database file is something you should be getting from RTTOV.
The namelist option:
rttov_sensor_db_file character(len=512) The location of the RTTOV sensor database. The format for the database is a comma-separated file. The columns of the database are the DART observation-kind, the platform/satellite/sensor ID, the observation type, the coefficient file, and a comma-separated list of RTTOV channels to use for this observation type.
looking at the code the channel list is optional:
! channel_list is an optional list of channels (can be zero-length, meaning all available channels should be used) there are no channels in the rttov_sensor_db.csv sensor file in the repo:
NOAA_1_VTPR1,1,1,7,ir,rtcoef_noaa_1_vtpr1.dat NOAA_2_VTPR1,1,2,7,ir,rtcoef_noaa_2_vtpr1.dat NOAA_3_VTPR1,1,3,7,ir,rtcoef_noaa_3_vtpr1.dat ...
are users supposed to edit rttov_sensor_db.csv or leave it as is?
The recommendation is to leave as is, and allow for all channels to be available, and the correct channel parameters are specified through the obs_seq.out channel metadata. Not clear to me if there would be any application where excluding certain channels through the DART database file would be appropriate.
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.
@hkershaw-brown, I think I addressed all your review suggestions. Check through these docs changes, and make sure the converter code doesn't give you any issues.
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.
looks great, approved.
todo: note in release notes, this will affect people using subsetted rttov files.
Description:
I removed the satellite channel offset within GOES ABI observation converter such that the wavelength in the obs_seq.out match that
within the RTTOV coefficient file.
I also improved the documentation both within the GOES obs converter and the RTTOV module to alert the user to potential
mismatch issues.
Fixes issue
#731
Types of changes
Documentation changes needed?
Tests
I have performed offline checks with the GOES obs converter, then used those obs_seq.out files to run filter
to confirm functionality.
Checklist for merging
Checklist for release
Testing Datasets