Enable restriction of an even-sphere obs network to a lon-lat box

kdraeder commented 1 year ago

Description:

This is a new version of even_sphere.m, modified to restrict the observations to a lon-lat box, but maintain the evenly-spaced pattern produced by the "golden section spiral". It was needed to create an obs network restricted to the Tropics for a Zagar OSSE. I generalized it to allow restrictions of longitude as well. I did some refactoring to reduce the number of similar loops and gather closely-related tasks.

Fixes issue

None (yet)

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[x] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
[x] Documentation update

Documentation changes needed?

[ ] My change requires a change to the documentation.
- [x] I have updated the documentation accordingly. The matlab script can print a "help" section, which now has an example of restricting the domain.

Tests

I specified a subdomain that spans the prime meridian and the equator and examined the picture of the profile locations.

Reviewers

I don't know who knows Matlab and would like to review this.

Checklist for merging

[ ] Updated changelog entry
[ ] Documentation updated
[ ] Update conf.py

Checklist for release

[ ] Merge into main
[ ] Create release from the main branch with appropriate tag
[ ] Delete feature-branch

Testing Datasets

[ ] Dataset needed for testing available upon request
[ ] Dataset download instructions included
[x] No dataset needed

hkershaw-brown commented 1 year ago

Hi @kdraeder I haven't looked too closely at this yet, but this looks like some added options to even_sphere.m.

Is the goal to replace the existing even_sphere.m so we have one .m file that can creates profiles on the whole sphere, or a restricted horizontal area of the sphere?

kdraeder commented 1 year ago

It could be a replacement, but I saw that there is already a variant; even_sphere_pe2lyr.m. So I thought maybe the philosophy here might be to have specialized scripts. There are no log entries for even_sphere_pe2lyr.m, so it's not clear. Generally I'm in favor of minimizing code and having fewer programs that do more things, but I've been outvoted in the past. So it's your call.

nancycollins commented 1 year ago

based on the lack of capitals in the comment at the top of the pe2lyr, i'd guess it's something i made. i'd vote to remove it from the repo. it's just an older duplicate of the basic program with only 2 levels in the vertical. it's not worth keeping, in my view. as long as the code for a restricted lat/lon box doesn't complicate the program so much that it's hard to follow, i'd vote for a single program here.

nancycollins commented 1 year ago

if all the box does is restrict the output to that box and doesn't change where the points are generated, then the obs_sequence_tool has a box option to cull obs outside the box. you could run the original code unchanged and then cut the obs down as a post processing step.

hkershaw-brown commented 1 year ago

@kdraeder let's put your changes into even_sphere.m
Leave even_sphere_pe2lyr.m as out of scope for this pull request

git mv even_sphere_subdom.m even_sphere.m should replace even_sphere.m with your new changes.

kdraeder commented 1 year ago

@nancycollins comment about obs_sequence_tool reminds me that in Manhattan we'll be using the preprocessor strategy for cam. I'd forgotten that, as I was importing this from the reanalysis branch where I needed it and there's no preprocessor step. Creating the global obs set and extracting a subset using obs_sequence_tool in the preprocessor step looks like the more consistent strategy.

So it appears to me now that we should leave even_sphere.m unchanged, ignore even_sphere_subdom.m, and close this PR as "won't do" (?). Sorry for the wasted effort.

hkershaw-brown commented 1 year ago

@kdraeder no problem, do you want to switch the PR from main to the reanalysis branch to have a record of what what used?

nancycollins commented 1 year ago

even_sphere used to be a simple matlab script. now it's a function - but you have to add a bunch of arguments to get it to run at all? that doesn't seem good. can't there be reasonable defaults in the script? i can't figure out how to run it at all. we should at least have an example matlab script that calls it with reasonable values. someone who knows too much matlab made this pretty hard to use, in my opinion. </end rant>

nancycollins commented 1 year ago

ok - i found you can run it with even_sphere(100) to gen 100 obs. but i tried restricting it with the lat/lon box and it didn't generate obs inside the box. the plot looks like the box you specified, but the locations in the output file aren't the same. so we definitely don't want this version on main, and if this isn't the exact version kevin used, you don't want it on the reanalysis branch either.

kdraeder commented 1 year ago

Agreed. It's broken. It's not needed. I'm abandoning it.

jlaucar commented 1 year ago

Kevin, Let's chat about this tomorrow. Let it go for this evening. Jeff

On Wed, Sep 6, 2023 at 3:58 PM kdraeder @.***> wrote:

Agreed. It's broken. It's not needed. I'm abandoning it.

— Reply to this email directly, view it on GitHub https://github.com/NCAR/DART/pull/538#issuecomment-1709183496, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANDHUIVOMZ5SAC4QW5LUJW3XZDW2FANCNFSM6AAAAAA4IHBRW4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

NCAR / DART