search

NCAR / DART

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

bug: obs_sequence_tool doesn't trim by location for 1d domains #547

Closed hsanter closed 10 months ago

hsanter commented 11 months ago

:bug:

Describe the bug

  1. List the steps someone needs to take to reproduce the bug.
  2. What was the expected outcome?
  3. What actually happened?
1. steps to reproduce: - for a 1d model: - specify min_box and max_box in obs_sequence_tool_nml (alongside other relevant namelist info e.g. file names) - run obs_sequence_tool 2. Expected outcome: output file from obs_sequence_tool only contains observations with locations between min_box and max_box 3. Actual outcome: output file doesn't remove observations outside of the specified interval. All other trimming (by time, type, metadata, etc.) is still done. Comment: Within obs_sequence_tool.f90, the boolean restrict_by_location is shared by the code for location trimming for the 1d and 2d case. Because of that, if no 2d bounding box is specified with min_lat, min_lon, max_lat, max_lon, then restrict_by_location is set to false, regardless of whether it was set to true previously based on min_box and max_box being set. Attempting to set values for the 1d and 2d case results appropriately in an error, so there is no way for restrict_by_location to be true by the time trimming occurs for a 1d model. Suggested fix: replace obs_sequence_tool.f90: line 306 `restrict_by_location = .false.` https://github.com/NCAR/DART/blob/6eeb4539f1d140c3184c2f9521a1b3bdf9d3396f/assimilation_code/programs/obs_sequence_tool/obs_sequence_tool.f90#L306 with ``` if(restrict_by_location) then continue else restrict_by_location - .false. endif ``` or something similar, so that restrict_by_location is not overridden if it was previously set to true at line 226. ### Error Message Please provide any error messages. ### Which model(s) are you working with? Lorenz 04 (though this should apply to all 1d models) ### Screenshots If applicable, add screenshots to help explain your problem. ### Version of DART Which version of DART are you using? You can find the version using `git describe --tags` Unsure ### Have you modified the DART code? Yes/No If your code changes are available on GitHub, please provide the repository. Yes, but not in obs_sequence_tool. ### Build information Please describe: 1. The machine you are running on (e.g. windows laptop, NCAR supercomputer Cheyenne). 5. The compiler you are using (e.g. gnu, intel). Compiling with gfortran on a UMD (red hat linux) server
nancycollins commented 11 months ago

good sleuthing. could the fix be as simple as just removing lines 305 and 306? restrict_by_location will have been set to true or false already in lines 224-231. if it's already true, just leave it alone?

edit: true, false - sigh, got them backwards in my first stab at this comment.

hsanter commented 11 months ago

I think that would work just as well, yup!

nancycollins commented 11 months ago

actually, for clarity i might suggest that the code set restrict_by_location to false around line 223, before the if block. then the following two code blocks can set it to true if they are entered, and do nothing if they are not. there are two alternate ways of setting the location limits and the logic might be more obvious if it was fixed this way.

hsanter commented 11 months ago

That sounds like a clearer fix - so in summary,

so that restrict_by_location is set to false by default, set to true if the 1d or 2d location inputs are provided, and we still don't allow both to be set because of the if block at lines 237-240.

On Tue, Sep 26, 2023 at 4:57 PM nancy collins @.***> wrote:

actually, for clarity i might suggest that the code set restrict_by_location to false around line 223, before the if block. then the following two code blocks can set it to true if they are entered, and do nothing if they are not. there are two alternate ways of setting the location limits and the logic might be more obvious if it was fixed this way.

— Reply to this email directly, view it on GitHub https://github.com/NCAR/DART/issues/547#issuecomment-1736414051, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALSHC2XKCPUZJBOJMCPCPBLX4NMTXANCNFSM6AAAAAA5IC6XBU . You are receiving this because you modified the open/close state.Message ID: @.***>

-- Henry Santer Department of Atmospheric and Oceanic Science University of Maryland, College Park (305) 562-0503

nancycollins commented 11 months ago

yep, that's it exactly.