Clean up and simplification of radiative transfer module#524
Conversation
|
Recording the discussion from yesterday's ISOFIT meeting. The following expression multiplies two quantities that are resampled from high spectral resolution:
A more accurate approach would calculate the reflected radiance transmitted along the bent (downwelling & upwelling) path, where the downward portion is separated into diffuse and direct terms and the upwelling portion is the total transmittance. Here rho_diffuse and rho_direct come from the surface model; they would be identical for Lambertian surfaces but differ for glint surfaces. This still suffers from some inaccuracy via division by the spherical albedo term, which could contain high spectral resolution structure. Xu Liu's team is investigating the impact of this approximation. |
|
Thanks for recording the outcome from yesterday's discussion, @davidraythompson! This is absolutely the right direction. However, I'd like to clarify the implementation of the adjacency effects. In Guanter et al. (2009), the direct upward transmittance is multiplied by the target reflectance, while the diffuse upward transmittance is multiplied by the background reflectance: |
|
Good point - if we want to model background reflectance independently we'd need to model four terms (all combinations of diffuse and direct on the downward and upward directions). Perhaps that is the best solution. |
|
Ok, sounds good! I'll work on a modified implementation that captures all the mentioned effects appropriately, i.e., that tries to minimize any inaccuracies due to multiplication of resampled quantities in the combined terms. |
| sky_glint = np.zeros(rfl.shape) | ||
|
|
||
| # adjacency effects | ||
| bg = geom.bg_rfl if geom.bg_rfl is not None else rfl |
There was a problem hiding this comment.
@pgbrodrick @davidraythompson In cases where we don't have geom.bg_rfl populated, should bg also contain the glint terms or should it just be rfl?
There was a problem hiding this comment.
Good question. Just thinking out loud...
- glint is likely (though not always) spatially autocorrelated
- we can't know whether or not it is
- if we do include it, I think it would come into lines 219/220 now, so as to continue to separate sun and sky glint into appropriate sections (sun w/ direct, sky with diffuse....I think?)
- technically, if the bg is different, we should be solving for two more terms (background sun & sky glint), though presumably with increasing degeneracy. At the limit though, we could enable the glint model to have 4 terms, which would be the most general way to handle it.
All that said, my default answer is probably to keep it glint-free, acknowledging that this creates a discrepancy when it's not true. What does @davidraythompson think?
There was a problem hiding this comment.
@davidraythompson Please let us know your thoughts about this!
There was a problem hiding this comment.
As discussed in our meeting, I think that it may be simplest to have bg_rfl be a single vector with n values for n spectrometer channels - the effective background reflectance quantity needed for our mulitple scattering calculations. We'll have to be careful about how we calculate this quantity, but we could start with the lambertian assumption (i.e. that the average directional reflectance we measure of the enclosing superpixel is the same everywhere). I hypothesize this would fail in the edge case of a large placid lake acting as a specular reflector, but should get better for more typical wavy surfaces; the distribution of surface facets becomes more uniform over larger areas, right?
There was a problem hiding this comment.
Agreed, that's what I would expect as well. Another option in the frame of the lambertian assumption might be to take the prior mean from the surface model as background reflectance. In case we run ISOFIT per pixel, we wouldn't have access to the average directional reflectance from the superpixel.
eac1568 to
cb94108
Compare
|
@pgbrodrick @davidraythompson @jammont @evan-greenbrg This PR is finally ready for your review! All checks are passing now and running some tests on AVIRIS-3 data gave me the exact same reflectance results as with the current |
Handling glint_model outside of templates fixed to handle topography model Optimized surface indexing, and added better logging for multisurface bug in abspath handling left the abspath statement in Removed surface dir support adding temporary handling of multi-transmittance luts adding command line support for class file in apply_oe Commenting out L592-597. Check with Niklas for utility of these lines Trying another fix for missing case2 Updated test_rfls and two_albedo method in line with isofit#524 update template construction for lut without obszen added custom surface model hash updated to example syntax keeping original n_core values constant add flag to pass directions for applyoe to run multipart_transmittance Cleaning up code
|
Overall this looks absolutely fantastic. There are a few places, noted in specific comments, where using variable names and terminology in a more consistent way might help clarify the physical interpretations of the different variables. |
|
Thanks for the comments, @davidraythompson! The use of variable names and terminology should be more consistent now. |
evan-greenbrg
left a comment
There was a problem hiding this comment.
@unbohn Here's a line-by-line review of the proposed changes. All the question I have here are very minor.
…-Strub et al. (2006).
561e5b8 to
5e22d7c
Compare
…th coupled terms assignment.
…th 1-c and 4-c models.
|
This Herculean effort is now ready for merge! Congratulations @unbohn and thanks to the full team. There may be small tweaks needed for additional optimization and performance. |
@pgbrodrick @davidraythompson This is a first draft for a simplification of
radiative_transfer.py. The two main goals are:transm_down_difonly holds the diffuse downward transmittance and not the total transmittance (down * up, direct + diffuse), such as in the current sRTMnet and standard MODTRAN case.There are two main problems/questions as of now:
transm_up_dirandtransm_up_difis not zero. Otherwise, the term((E_down_dir + E_down_dif) * (T_up_dir + T_up_dif)) / (1.0 - s_alb * bg)would be zero as well causing incorrectly modeled radiance. With sRTMnet and standard MODTRAN we currently don't have separated upward transmittance available. Proposed solution: deprecate single-transmittance mode in ISOFIT.transm_up_dirandtransm_up_difwould be zero. Proposed solution: train emulator and build LUT to output separated downward and upward fluxes.Please find some more topics for discussion at in-line comments.
Let me know what you think!