Skip to content
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

Question about atl06p processing params for multiple outputs #448

Open
bpurinton opened this issue Nov 19, 2024 · 3 comments
Open

Question about atl06p processing params for multiple outputs #448

bpurinton opened this issue Nov 19, 2024 · 3 comments

Comments

@bpurinton
Copy link

I have a method in a package I'm building that looks like this:

def pull_atl06sr_multi_processing(
        self,
        res=20,
        len=40,
        ats=20,
        cnt=10,
        maxi=5,
        # ...
    ):
        # region to pull from
        region = Raster(self.dem_fn).get_bounds(latlon=True)

        parms_dict = {
            "high_confidence": {
                "cnf": 4,
                "srt": 3,
            },
            "ground": {
                "cnf": 0,
                "srt": 0,
                "atl08_class": "atl08_ground",
            },
            "canopy": {
                "cnf": 0,
                "srt": 0,
                "atl08_class": "atl08_canopy",
            },
            "top_of_canopy": {
                "cnf": 0,
                "srt": 0,
                "atl08_class": "atl08_top_of_canopy",
            },
        }

        for key, parms in parms_dict.items():
            parms["poly"] = region
            parms["res"] = res
            parms["len"] = len
            parms["ats"] = ats
            parms["cnt"] = cnt
            parms["maxi"] = maxi
            parms["samples"] = {
                "esa_worldcover": {
                    "asset": "esa-worldcover-10meter",
                }
            }
           # ...
            atl06sr = icesat2.atl06p(parms)
            # ...
            self.atl06sr_processing_levels[key] = atl06sr

Ultimately, what I'm trying to do here, is gather four processing levels of ATL06: "high_confidence", "ground", "canopy", and "top_of_canopy". The method also accepts the important fitting parameters res, len, cnt, etc. that are applied to all four processing levels, should the user want to tweak those (but they default to the ATL06 defaults).

I built up the four processing levels, e.g.

           "ground": {
                "cnf": 0,
                "srt": 0,
                "atl08_class": "atl08_ground",
            },

By carefully reading the SlideRule ICESat-2 docs page, and concluding that I needed a combination of srt, cnf, and atl08_class parameters as specified in my parms_dict variable.

However, I'm still not 100% sure these are the four combinations of processing parameters that will return exactly what I want (high confidence ground; and ATL08-classified ground, canopy, and top of canopy). In particular, I'm not sure I want to use that atl08_class parameter for this use case.

For instance, I know there are also the PhoREAL and the yapc parameters, which might also get at what I'm after with this multi-processing.

Am I on the right track with my parameter selection above? cc @dshean

Thanks!

@bpurinton
Copy link
Author

As an update, after playing around with the SR demo client, I refined my point selection to:

        parms_dict = {
            "high_confidence": {
                "cnf": 4,
                "srt": -1,
            },
            "ground": {
                "cnf": 0,
                "srt": -1,
                "atl08_class": "atl08_ground",
            },
            "canopy": {
                "cnf": 0,
                "srt": -1,
                "atl08_class": "atl08_canopy",
            },
            "top_of_canopy": {
                "cnf": 0,
                "srt": -1,
                "atl08_class": "atl08_top_of_canopy",
            },
        }

The only difference here being the "srt" changed to -1 in all cases. I'm not sure I understand this -1 value? It seems to correspond to the icesat2.SRT_DYNAMIC constant, but I can't find documentation about that anywhere.

@jpswinski
Copy link
Member

Hi Ben, what you are doing looks right to me. Here are my comments:

  • The SRT being set to -1 is a good idea; it is something we've added recently but hasn't been documented because we are woefully behind on documentation. What it means is it tells the server side code to look at the ATL03 confidence array for each photon and choose the confidence level that is highest across all five surface type entries. We've found that it is not only inconvenient to always need to match the surface type to the region being processed in the request, but sometimes for large requests, it is not possible since regions may span multiple surface types. This does away with needing to worry about it because it takes the highest confidence regardless of the surface type.

  • The esa-worldcover-10meter has given us some issues recently; please let me know if you see any issues and we will look into it more than we have. They were transient about a month ago and I haven't had time to revisit it, but we will definitely revisit it if you see issues.

  • I think the four sets of parameters make sense for the four things you are trying to target. But I wanted to point out that for the high confidence ground - it is not necessarily the case that you will get ground. You'd have to talk to Jeff Lee if you want more details, but I believe the confidence is only related to whether or not it is a surface reflection, regardless of what that surface is.

  • Please let me know how it goes!

@bpurinton
Copy link
Author

bpurinton commented Nov 27, 2024

The SRT being set to -1 is a good idea ... This does away with needing to worry about it because it takes the highest confidence regardless of the surface type.

Great! I kind of interpreted that from my reading online but just wanted to be sure I understood

The esa-worldcover-10meter has given us some issues recently;

No issues with this from my work thus far!

I wanted to point out that for the high confidence ground - it is not necessarily the case that you will get ground. You'd have to talk to Jeff Lee if you want more details, but I believe the confidence is only related to whether or not it is a surface reflection, regardless of what that surface is.

Yes, I was not that convinced by my "high_confidence" parameters, realizing that these are not necessarily "ground" but just any "surface" returns, which might mix land surface types (veg, snow + ice, etc.), and think I will likely drop that in favor of the three atl08-based requests, which better target and separate the points into clear classes for further analysis. Thanks for confirming my thoughts there.

bpurinton added a commit to uw-cryo/asp_plot that referenced this issue Jan 16, 2025
For a discussion of this, see: SlideRuleEarth/sliderule#448.

The main point is that "high_confidence" points are not necessarily a given surface type. Instead, we want to focus on just getting the ground, canopy, and top of canopy, which we already do.

This commit removes all traces of "high_confidence" points from the codebase.

In addition, this commit speeds up the altimetry tests by skipping the actual external retrieval call. This means the `request_atl06sr_multi_processing` function is not tested, but that is mostly a wrapper for sliderule anyway.

Tests are passing locally. Failing tests on CI may have to do with the CI setup on GitHub.
bpurinton added a commit to uw-cryo/asp_plot that referenced this issue Jan 16, 2025
This commit removes all traces of "high_confidence" points from the codebase. For a discussion of this, see: SlideRuleEarth/sliderule#448.

The main point is that "high_confidence" points are not necessarily a given surface type. Instead, we want to focus on just getting the ground, canopy, and top of canopy, which we already do.

In addition, this commit speeds up the altimetry tests by skipping the actual external retrieval call. This means the `request_atl06sr_multi_processing` function is not tested, but that is mostly a wrapper for sliderule anyway.

Also, I fixed CI tests here, by dropping use of mamba in favor of conda, which seems to have fixed things.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants