search

NCAR / DART

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

duplication of print_obs_seq, validate_obs_seq_time, print_metadata #185

Open hkershaw-brown opened 3 years ago

hkershaw-brown commented 3 years ago

obs_info.f90 (program) has a print_obs_seq that is not called.

There is a print_obs_seq in obs_seq_utilities_mod.f90 that is public. That one has kinds in the variable names, not types.

There are 10 versions of print_obs_seq, which I think is about 9 too many:

./assimilation_code/programs/obs_utils/obs_sort.f90:535:subroutine print_obs_seq(seq_in, filename)
./assimilation_code/programs/obs_utils/obs_info.f90:305:subroutine print_obs_seq(seq_in, filename)
./assimilation_code/programs/obs_utils/obs_data_denial.f90:314:subroutine print_obs_seq(seq_in, filename)
./assimilation_code/programs/obs_utils/obs_remove_dups.f90:551:subroutine print_obs_seq(seq_in, filename)
./assimilation_code/programs/obs_selection/obs_selection.f90:675:subroutine print_obs_seq(seq_in, filename)
./assimilation_code/programs/obs_sequence_tool/obs_sequence_tool.f90:1149:subroutine print_obs_seq(seq_in, filename)
./assimilation_code/programs/obs_keep_a_few/obs_keep_a_few.f90:303:subroutine print_obs_seq(seq_in, filename)
./assimilation_code/programs/obs_common_subset/obs_common_subset.f90:713:subroutine print_obs_seq(seq_in, filename)
./developer_tests/obs_sequence/obs_rwtest.f90:190:subroutine print_obs_seq(seq_in, filename)
./observations/obs_converters/utilities/obs_seq_utilities_mod.f90:82:subroutine print_obs_seq(seq, filename)

The obs_seq_utilites_mod.f90 I don't think is used at all since it is not in any pathnames files. Aim: look at the various obs_seq_tools and whether these obs_seq programs should be using obs_seq_utitilies_mod.

nancycollins commented 3 years ago

i see you're way ahead of me.

timhoar commented 3 years ago

The print_obs_seq_summary() routine could use an optional argument to print some sort of header information/banner. In the AIRS/convert_amsua_L1.f90 program (for example) it is useful to call it to report on what is in each input file, and then again for the total sequence. Without a banner to identify what is coming next, the reports (at first blush) look redundant.

hkershaw-brown commented 2 years ago

13 validate_obs_seq_time

 grep -rn "subroutine validate_obs_seq_time(seq, filename)" .
./assimilation_code/programs/obs_info/obs_info.f90:340:subroutine validate_obs_seq_time(seq, filename)
./assimilation_code/programs/obs_assim_count/obs_assim_count.f90:527:subroutine validate_obs_seq_time(seq, filename)
./assimilation_code/programs/obs_selection/obs_selection.f90:787:subroutine validate_obs_seq_time(seq, filename)
./assimilation_code/programs/obs_sequence_tool/obs_sequence_tool.f90:1269:subroutine validate_obs_seq_time(seq, filename)
./assimilation_code/programs/obs_data_denial/obs_data_denial.f90:434:subroutine validate_obs_seq_time(seq, filename)
./assimilation_code/programs/obs_remove_dups/obs_remove_dups.f90:667:subroutine validate_obs_seq_time(seq, filename)
./assimilation_code/programs/obs_sort/obs_sort.f90:650:subroutine validate_obs_seq_time(seq, filename)
./assimilation_code/programs/obs_total_error/obs_total_error.f90:368:subroutine validate_obs_seq_time(seq, filename)
./assimilation_code/programs/obs_keep_a_few/obs_keep_a_few.f90:423:subroutine validate_obs_seq_time(seq, filename)
./assimilation_code/programs/obs_common_subset/obs_common_subset.f90:834:subroutine validate_obs_seq_time(seq, filename)
./assimilation_code/modules/observations/obs_sequence_mod.f90:2908:subroutine validate_obs_seq_time(seq, filename)
./developer_tests/obs_sequence/obs_rwtest.f90:309:subroutine validate_obs_seq_time(seq, filename)
./observations/obs_converters/utilities/obs_seq_utilities_mod.f90:204:subroutine validate_obs_seq_time(seq, filename)
hkershaw-brown commented 2 years ago

There is a note in all the validate_obs_seq_times:

https://github.com/NCAR/DART/blob/0de0a8aa25ddbec49b3b278a19ad4136eb829322/assimilation_code/programs/obs_info/obs_info.f90#L340-L349

I think this note (x13) is talking about this 2009 fix: 5727c3f7e33a5df7cd9acfd180b371f36e8ab124 Double check this.

nancycollins commented 2 years ago

i also remember at least one case where a user was creating their own obs_seq files with python code and didn't have the linked lists implemented correctly. this made filter unhappy in a hard-to-diagnose way. (by the way, having 13 copies of the validate routine is me being lazy. i'm sure there are about as many print seq routines, also. bad nancy.)

hkershaw-brown commented 1 year ago

note obs_seq_utilites_mod is only available to observation converters at the moment buildconvfunctions.sh https://github.com/NCAR/DART/blob/70e6af803a52d14b9f77f872c94b1fe11d5dc2d9/build_templates/buildconvfunctions.sh#L118

Edit: correction obs_seq_utilies_mod is not available to any code, and has not been updated (or maybe ever used)

e.g max_obs_kinds does not exist in obs_def_mod.f90 https://github.com/NCAR/DART/blob/70e6af803a52d14b9f77f872c94b1fe11d5dc2d9/observations/obs_converters/utilities/obs_seq_utilities_mod.f90#L33C62-L33C62