cosi patch#863
Conversation
|
Here's the scene that I ran for #854 , run with setting the min cos(i) value to 0. Looks like this PR helps, but isn't enough to bring things inline: |
|
Also included in this PR is the slight modification to the E.g., cosi_min of 0.3 for a scene with a high sun in the sky only happens on cliffs essentially. But for say a low sun, this can happen on milder slopes. This change does not force us to read slope and aspect data from OBS, because we know the minimum cosi will occur when SAA - Aspect is equal to 180 degrees, and slope= Below is a couple examples of computing a min cos_i to help determine a max slope to use as a default. min cosi table
|
|
📊 Generated results:
URL: isofit/isofit-test-results#9 |
|
There is currently a small bug where But hard coding just to try a few runs, and I think we should even go with a more conservative slope threshold. I would suggest somewhere around 30-35 deg based on this EMIT scene and the 60 m DEM we are using ... Even at 45 deg, there is still high potential to have some large outliers. Note that 45 before was selected by me sort of arbitrarily! :) Once again just as a note, the max_slope is effectively informing our min_cosi , but NOT the max_cosi as this type of error typically is less catastrophic on the other side of the coin. 
|
|
Implemented and tested (not exactly 1:1 with other lines on this plot...e.g. includes CO2 in statevector...but very close). The same test pixel still shows bad behavior...but looking at the slope and it happens to be the stressing case of just under 30 degrees (28.xx) 
Plotting the reflectance at 1600 nm (none of which should exceed 1 for the scene) against the slope gives: From this in isolation, I'd want to bring the max down to 25 or maybe even 20....but that feels a bit low. Obviously it's tricky because misregistration is likely a very heavy player here. |
|
@pgbrodrick This is very interesting! Since the slope threshold only is used for shaded slopes , I don't think 20deg would be unreasonable. The number here like you say is going to be a function of DEM accuracy (both in x-y co-reg and z). So this may be smart to bring it down further than the 30deg for EMIT+SRTM-DEM based on test results you are finding. |
…, test_geometry.py)
|
From my point of view, this one is ready to go. It already provides a small clean up of |
| bg_rfl: np.array = None, | ||
| svf: float = 1, | ||
| coszen: float = None, | ||
| full_config: configs.Config = {}, |
There was a problem hiding this comment.
I don't love having the full config as an input here, but it's in style with the other isofit objects.
I think the ways around it may end up being more complicated with the current geometry structure. Could be worth thinking about adding a Geometry section to the isofit config to navigate the hard-coded constants. There may also be solutions that become more accessible when revisiting the IO. My guess is that file type flexibility will include some object-based meas and geom abstraction.
The LUT-grid check is a real challenge. This is a code architecture question more so than any real concern with consistency. I think in that spirit, it shouldn't hold up the PR.
There was a problem hiding this comment.
Agreed @evan-greenbrg . with this PR now in, I believe max_slope and terrain_style are only touched here and template construction. and so could definitely be a geom config section? I could see it becoming even nicer too as we have more complex geometry (e.g., svf, and the point you saw about the cos_i_bg)..
And then also agree about the coszen check. I think on some level, these have to be checked?... but where, is hard especially with the upcoming changes! :)
This fix changes the cosi of the background to be different than the target pixel and more closely follows EQ 6.18-6.20 of ATCOR ATBD. Note the line that mentions the averaging of V_t (i.e., skyview) in 6.19.
Testing on EMIT image over Patagonia this has been shown to reduce spurious outliers in reflectance. For now, this assumes the
cos_iof the background to be the same as the solar incidence angle at TOA. In the future, we may consider to aggregatecos_iover some adjacency range to populate these new termscos_i_bgandskyview_factor_bg.