Skip to content

Merge RT/Engines, LUT structure#953

Merged
pgbrodrick merged 29 commits into
isofit:rte_refactorfrom
evan-greenbrg:refactor/rte
May 5, 2026
Merged

Merge RT/Engines, LUT structure#953
pgbrodrick merged 29 commits into
isofit:rte_refactorfrom
evan-greenbrg:refactor/rte

Conversation

@evan-greenbrg

@evan-greenbrg evan-greenbrg commented Apr 27, 2026

Copy link
Copy Markdown
Collaborator

I pushed up draft of the LUT-atmosphere architecture. I drew up an accompanying diagram.
In short:

  1. the idea is that the Atmosphere object that forward holds onto is most analogous to the previous engine object.
  2. The base object, Atmosphere , inherits the LUT reader
  3. The engine wrappers inherit Atmosphere , and the LUT reader via the Atmosphere, and the LUT writer
  4. The atmosphere construction, i.e. engine, is constructed in forward.py with a call to atmosphere.init .
  5. LUTs are still built/loaded at initialization via an overloaded function, _lut. The base function within Atmosphere , just loads the LUT. The Engines contain a function with the same name, but will first call the LUT Writer before
    loading.
  6. One change that we can discuss is that the actual LUT is passed around within a LUT object, which can be called directly via a LUT.call .

That's the idea anyway. Still need to work through debugging.

LUT_architecture

Status:

Cleanup and testing

Comment thread isofit/atmosphere/atmosphere.py
Comment thread isofit/core/forward.py Outdated
Comment thread isofit/atmosphere/atmosphere.py Outdated
pairs = zip(self.lut_names, x_RT)
return " ".join([f"{name}={val:5.3f}" for name, val in pairs])

# REVIEW: We need to think about the best place for the two albedo method (here, radiative_transfer.py, utils, etc.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would motion to remove this review note, and that I think it sounded last week like this atmosphere.py was the best place.

Comment on lines -577 to -582
# convert observer zenith to MODTRAN convention if needed
if self.indices.convert_observer_zenith:
point[self.indices.convert_observer_zenith] = (
180.0 - point[self.indices.convert_observer_zenith]
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double check this should/does still exist for the Modtran case? Also, was looking for it because this would be a nice one to add to units.py too I think.

Comment thread isofit/atmosphere/atmosphere.py Outdated
@pgbrodrick

Copy link
Copy Markdown
Collaborator

Huge progress! Posted small patch PR here from items initially caught: https://github.com/evan-greenbrg/isofit/compare/refactor/rte...pgbrodrick:isofit:rte_patches?expand=1

(sorry, forgot to suppress black). Small changes worth double checking, but I think they will help. Done prior to 13f9eba.

@pgbrodrick

pgbrodrick commented Apr 29, 2026

Copy link
Copy Markdown
Collaborator

More work to get the small cube example to run to completion (yay) in evan-greenbrg#7

@brentwilder , can you review? Particularly the question on coszen in fileio (will examine with fresh eyes tomorrow). Evan noted he's okay with direct pushes, here, but I'd prefer some quick checks on these first.

Now merged.

Comment thread isofit/core/fileio.py
Comment on lines +461 to +462
# coszen stored as scalar in the LUT
self.coszen = float(forward.atmosphere.lut["coszen"].values)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This similar line needs to be included in analytical_line.py and algebraic_line.py

Comment thread isofit/configs/sections/atmosphere_config.py
Comment thread isofit/atmosphere/engines/libradtran.py
Comment on lines -224 to -227
self.glint_model = engine_config.glint_model
if self.glint_model and not self.multipart_transmittance:
raise AttributeError(
"Using the glint model requires a multipart transmittance LUT table"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with the config, glint_model gone in this update, do we need a different guardrail to prevent user from running glint without multipart? Or perhaps this removed because we already check this somewhere else in the code?

@pgbrodrick

Copy link
Copy Markdown
Collaborator

More test help on evan-greenbrg#8 - if we're okay with the backwards compatibility check, this should be good to go.

@evan-greenbrg evan-greenbrg marked this pull request as ready for review May 4, 2026 18:06
@pgbrodrick pgbrodrick merged commit e2967c7 into isofit:rte_refactor May 5, 2026
17 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants