Skip to content

Srtm/6c#717

Merged
unbohn merged 30 commits into
isofit:devfrom
pgbrodrick:srtm/6c
Jul 9, 2025
Merged

Srtm/6c#717
unbohn merged 30 commits into
isofit:devfrom
pgbrodrick:srtm/6c

Conversation

@pgbrodrick

Copy link
Copy Markdown
Collaborator

To replace #662. To be merged after #713.

@pgbrodrick pgbrodrick mentioned this pull request Jun 19, 2025
@pgbrodrick

Copy link
Copy Markdown
Collaborator Author

Current failure is for legacy (1c) emulator, needs a key fix. There also appears to be an issue with the initial guess, our perpetual challenge:

Screenshot 2025-06-19 at 3 53 05 PM

@pgbrodrick

Copy link
Copy Markdown
Collaborator Author

The offending issue is here:

https://github.com/pgbrodrick/isofit/blob/29b0a6ddb125ed0b25784f305558720c37a16945/isofit/inversion/inverse_simple.py#L115-L122

This product clearly won't work in 4c radiance mode, only 4c transmittance.

# Keep transmittances as transmitance, but convert coupling terms to radiance
# TODO - get rhoatm out of this list. Probably easiest to implement a bulk
# rt_mode purge
for key in ["dir-dir", "dir-dif", "dif-dir", "dif-dif", "rhoatm"]:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is it worth renaming these ("dir-dir") to "Ldir-dir", etc?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We could, we'd have to link that change to what's in RTE right now as well, as that's where the coupling is defined...this just matches that nomenclature.

@evan-greenbrg

Copy link
Copy Markdown
Collaborator

Out of the box, simulations on the 6c emulator were taking an extremely long time:

INFO:2025-06-26,17:44:45 || common.py:__call__() |  20.02% simulations complete (elapsed: 0:28:10.708086, rate: 0:00:00.457443, eta: 1:52:42.832344)

Working this morning to confirm what the root cause is. Need to check if it's in the code or something on my end.

@pgbrodrick

Copy link
Copy Markdown
Collaborator Author

Hmm, how big is your LUT? I'm seeing numbers that look more like this:

INFO:2025-06-19,14:54:31 || common.py:call() | 20.06% simulations complete (elapsed: 0:00:17.743084, rate: 0:00:00.012802, eta: 0:01:10.972336)

For a LUT that looks like this:

"lut_grid": {
                "AOT550": [
                    0.001,
                    0.1009,
                    0.2008,
                    0.3007,
                    0.4006,
                    0.5005,
                    0.6004,
                    0.7003,
                    0.8002,
                    0.9001,
                    1.0
                ],
                "H2OSTR": [
                    0.4334,
                    0.6766,
                    0.9197
                ],
                "observer_zenith": [
                    4.5981,
                    8.2908
                ],
                "relative_azimuth": [
                    37.6767,
                    90.2157,
                    142.7548
                ],
                "surface_elevation_km": [
                    0.4898,
                    0.7043,
                    0.9189,
                    1.1334,
                    1.348,
                    1.5625,
                    1.7771
                ]
                }

There's definitely room for optimization (e.g. parallelize deployment over different keys), and I do expect this PR to take longer than before no matter what (we're building close to an order of magnitude more outputs), but the above looks like it's way too slow....almost like it's before I included the updates to utilize the cached H matrix.

@evan-greenbrg

evan-greenbrg commented Jun 30, 2025

Copy link
Copy Markdown
Collaborator

I re-ran and run times were much faster:

INFO:2025-06-30,10:58:45 || common.py:__call__() |  90.00% simulations complete (elapsed: 0:07:47.878095, rate: 0:00:00.106336, eta: 0:00:51.986455)

Not exactly sure what the cause was. Profiling the LUT generation for a small LUT grid (size as a product of len(lut grid element): 2 * 2 * 2* 2 * 2) also confirms that things are probably running as expected. Bulk of time is the 6s simulation. The unlabeled chunk on the right are numpy array IO operations for the weight/bias matrices.

Screenshot 2025-06-30 at 11 00 28 AM

@pgbrodrick

Copy link
Copy Markdown
Collaborator Author

Current emulators FOR TESTING ONLY - NOT OFFICIAL: https://popo.jpl.nasa.gov/pub/PBrodrick/isofit/experimental/joint_dataset_wpoints/

@pgbrodrick pgbrodrick mentioned this pull request Jul 2, 2025
@davidraythompson

Copy link
Copy Markdown
Collaborator

Thank you Phil! FYI, using *26.npz I'm getting the error:
KeyError: 'weights_rhoatm is not a file in the archive'
stacktrace.txt

@pgbrodrick

Copy link
Copy Markdown
Collaborator Author

Thank you Phil! FYI, using *26.npz I'm getting the error: KeyError: 'weights_rhoatm is not a file in the archive' stacktrace.txt

Did you pull the file fresh today? It was reposted in-place (with the same name). That error will occur with the previous packed version of the .npz.

@jammont jammont left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just a few fairly minor comments. Looks pretty good.

Comment thread isofit/core/common.py Outdated
Comment thread isofit/radiative_transfer/engines/sRTMnet.py Outdated
Comment thread isofit/radiative_transfer/engines/sRTMnet.py Outdated
Comment thread isofit/radiative_transfer/engines/sRTMnet.py Outdated
Comment thread isofit/radiative_transfer/engines/sRTMnet.py
from isofit.utils import surface_model


# TODO - re-engage this test w/ 6c emulator & new glint model

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When will this test be re-enabled?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Agreed this should happen, but I don't think it's necessary in this PR (it will take some work, and we should wait for an official sRTMnet 6c model posting.

@unbohn

unbohn commented Jul 9, 2025

Copy link
Copy Markdown
Collaborator

This is ready to go into dev after extensive testing and review. Thanks, @pgbrodrick! Merging now.

@unbohn unbohn merged commit b14dc96 into isofit:dev Jul 9, 2025
17 checks passed
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

Successfully merging this pull request may close these issues.

5 participants