DOI-USGS / DOI-USGS/ISIS3

HapkeLROC silently uses the wrong filter's photometric parameters (single-band and multi-band FROM cubes both affected)

Open
#6,157 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
245
Forks
181
Avg merge
1d 22h
Merged PRs (30d)
15

Description

## Summary

`HapkeLROC` (used by `lrowacphomap`) silently applies the **wrong filter's** photometric parameters when its `PHOALGO` PVL defines multiple `Parameters` groups (e.g. the standard 7-filter `WAC_hapke_full.0001.pvl`), for both of the two ways we tried to use it — a single-band `FROM` cube with a backplane (the pattern the app's own documentation shows), and a genuinely multi-band `FROM` cube. In both cases the output is numerically wrong, not just mislabeled: we confirmed by hand-deriving the correct value from the same formula and parameter cube that the tool returns a different filter's answer instead.

This was found while trying to explain a large (~2.7-3x), previously unexplained systematic low bias between our own Hapke-normalized WAC mosaics and the official archived WAC_HAPKE RDR product. This specific bug turned out not to be the cause of that particular bias (the two filters involved, 643 nm and 689 nm, happen to have numerically similar parameters at the pixel we traced), but it's a real, independently-confirmed defect that corrupts results whenever a recipe's bands differ more (it's severe for 415 nm, whose true parameters differ substantially from the incorrectly-substituted 689 nm ones).

**Environment**: ISIS 10.0.0 (conda `isis10.0.0` package). Verified the same code is present on `dev` at commit `2acd3cfe5c7e323f49e6bc561bd36f130c4852b4` (`isis/src/lro/apps/lrowacphomap/HapkeLROC.cpp` / `.h`, byte-identical to the 10.0.0 tag).

## Root cause 1: `Parameters::band` default value collides with a real band index

`HapkeLROC.h`:

```cpp
class Parameters {
public:
Parameters()
: band(1), bandBinCenter(0.0), mapBands(),
names(), phoStd(0.0), values() {}
...
int band; // band number
```

The default value of `band` is **1**, not an unmatched sentinel like `-1`.

In the constructor (`HapkeLROC.cpp`), one `Parameters` object is built per `Parameters` PVL group, and `band` is only *conditionally* overwritten:

```cpp
for (int j = 0; j < center.size(); j++) {
if (center[j] == paramGroup.findKeyword("BandBinCenter")[0]) {
parms.band = j + 1;
}
}
```

`center` comes from the `FROM` cube's own `BandBin/Center`. For a single-band cube (`center.size() == 1`), only one PVL group will ever satisfy this `if`; every other group's `band` is left at the untouched default of **1** — indistinguishable from a genuine match at `j == 0`.

## Root cause 2: no `break`/uniqueness check in the selection loop, so the *last* match wins

`HapkeLROC::photometry(i, e, g, lat, lon, band)`:

```cpp
for (unsigned int p = 0; p < m_bandParameters.size(); p++) {
HapkeLROC::Parameters &parms = m_bandParameters[p];
if (parms.band == band) {
m_currentMapIndex = p;
for (unsigned int v = 0; v < parms.values.size(); v++) {
parms.values[v] = b[parms.mapBands[v]];
...
}
parms.phoStd = photometry(parms, m_iRef, m_eRef, m_gRef);
}
}
```

Given root cause 1, *every* group in a 7-filter PVL satisfies `parms.band == band` (`band` is always 1 for a single-band `FROM` cube). With no `break`, this loop runs to completion and `m_currentMapIndex` ends up pointing at whichever `Parameters` group is **last** in the PVL — 689 nm in the standard `WAC_hapke_full.0001.pvl` — regardless of which physical filter is actually being processed.

## Empirical confirmation

Using `WAC_hapke_full.0001.pvl` + `WAC_global_7bands_1x1_wbhs70NS_const_each_pole.0001.cub` against a real single-band-extracted WAC COLOR product cube + phocube backplane (`lrowacphomap ... normalized=false photometryonly=true`, i.e. asking for the raw, un-normalized reflectance value directly):

| Band (filter) | Own true parameters (hand-derived from the same cube+formula) | ISIS's actual output |
|---|---|---|
| 415 nm | `w=0.2405 b=0.2366 c=0.3337 ... theta=23.6566` -> **r = 0.07135** | **0.13144413** |
| 689 nm's parameters at band-1's geometry | `w=0.4253 b=0.2366 c=0.3345 ...` -> **r = 0.13144118** | (matches) |

ISIS's actual output for band 1 (415 nm) matches the *689 nm* parameter set applied to band 1's geometry to 5 decimal places, and is off by ~1.8x from the value its own true 415 nm parameters produce. This isn't a rounding or geometry difference — it's the wrong coefficient set entirely.

## Workarounds tried

**(A) Feed `lrowacphomap` a genuinely multi-band `FROM` cube instead of a single-band one** (so `center.size()` matches the number of real bands, avoiding the root-cause-1 collision for the bands actually present) — **does not fix it**. Running one `lrowacphomap` call on a real 5-band cube (`BandBin/Center = (415, 566, 604, 643, 689)`, no backplane, `usedem=false`, `normalized=false photometryonly=true`) still returns near-identical values across spectrally very different bands:

| Band | Output |
|---|---|
| 1 (415 nm) | 0.13117976 |
| 4 (643 nm) | 0.12793887 |
| 5 (689 nm) | 0.12654394 |

Despite `w` genuinely ranging 0.24 -> 0.395 -> 0.425 across these three filters at this location, the outputs vary by only ~4%. This points to a **second, compounding bug**: the outer per-pixel cache in `photometry(i, e, g, lat, lon, band)`,

```cpp
if (m_currentMapSample != intSamp || m_currentMapLine != intLine) {
...
for (p...) { if (parms.band == band) { m_currentMapIndex = p; ... } }
}
double ph = photometry(m_bandParameters[m_currentMapIndex], i, e, g);
```

is keyed **only** on the parameter map's pixel location (derived from lat/lon), not on `band`. For a multi-band cube processed one ground location at a time across all its bands, whichever band's parameters get loaded first for that location are silently reused for every other band at the same location, even though each band's `Parameters` entry was constructed correctly. So even fixing root causes 1 and 2 above would not be sufficient on its own for genuinely multi-band input — this cache would also need to be invalidated on a `band` change, not just a location change.

**(B) Build a per-band-filtered copy of the `PHOALGO` PVL, containing only the one `Parameters` group matching the band being processed** — **this works**. With only one `Parameters` group in the file, there's nothing else for the loop to (mis)match against. Re-running band 1 (415 nm) through the normal single-band + backplane pipeline with this filtered PVL:

```
custom per-band PVL, band1 (415nm): 0.071374774
```

This matches the independently hand-derived correct value (0.071348, using the exact same formula transcribed from `HapkeLROC.cpp`) to 4 significant figures, versus the buggy full-PVL result of 0.131441 — roughly 1.8x different. This is a practical, verified workaround for anyone hitting this with a single-band + backplane pipeline (our own use case), though it obviously doesn't help the multi-band case, which needs an actual code fix for root cause 3.

## Suggested fix

- Give `Parameters::band` a real unmatched sentinel (e.g. `-1`), not `1`.
- Either `break` after the first match in the selection loop, or (better) assert/throw if more than one `Parameters` group matches a given `band`, so a future version of this bug fails loudly instead of silently returning a plausible-looking but wrong number.
- Key the `m_currentMapSample`/`m_currentMapLine` cache invalidation on `band` as well as location, so multi-band `FROM` cubes are handled correctly too.

## Reproduction

Happy to provide the exact test cubes/PVLs/commands used above if useful — they're small, synthetic single-pixel-region ISIS cubes built from a real LRO WAC COLOR product plus the standard `$ISISDATA/lro/calibration/WAC_hapke_full.0001.pvl` and its paired parameter map cube, so this should reproduce with any real WAC COLOR product.

---

*This investigation (source reading, formula transcription/cross-checking, and the empirical tests above) was carried out with the assistance of Claude Code (Anthropic's AI coding agent). All numeric results were independently verified against the real `lrowacphomap` binary's output, not just derived from source reading.*

Contributor guide

Open the contributing guide

Research direction

Start in isis/src/lro/apps/lrowacphomap/HapkeLROC.h and HapkeLROC.cpp, reviewing Parameters construction and the photometry cache and selection loops. Reproduce the single-band and multi-band cases with lrowacphomap and the WAC calibration PVL/cubes described in the issue; done means each band uses its own parameters and the outputs match independently derived values.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
computer-vision
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.