Merged the IR and Vis uncertainty changes - #144
Open
mraspaud wants to merge 127 commits into
Open
Conversation
…odule which merges the two uncertainty outputs into a single xarray output.
Fixed issue causing NaNs in uncertainty output
…ible # Conflicts: # pygac/calibration/vis_uncertainty.py # pygac/klm_reader.py # pygac/pod_reader.py # pygac/reader.py # pygac/utils.py
Fixed issue of NaNs appearing in uncertainty output
…er than
calibrated values ("counts" rather than "channels")
…tions (before/after solZA max)
…ratio out of limits
…anck function variations but adding 0.5K@300K in radiance space for this component
…uses radiance at 300K assuming uncertainty of 0.5K) and updates to data files with new solar contamination routine.
write_solar option to output data to be used in unit testing
1) Added test for updated PRT numbering module 2) Added test for aspects of the IR uncertainty module including solar contamination of the gain, calculation of the min PRT standard deviation gain and calculation of the ICT uncertainty. Uses data from an orbit of real AVHRR data stored in associated CSV file.
Member
Author
|
@jmittaz looks like matplotlib is still imported somewhere where the tests are running, since the run crashes. |
… 3B')
Replace monolithic get_noise() (10-tuple return, twelve_micron flag, 3
channels in one function) with two single-responsibility helpers:
- _channel_noise(space_2d, ict_2d, bad_scans, window)
One channel. Formula: sqrt(allan_dev^2 + 1/3), divided by sqrt(window*10).
Returns (noise, av_noise, av_ict_noise) — no flags, no branching.
- _compute_bad_scans(specs, total_space, total_ict)
Shared scan-quality mask. Vectorised with np.stack + .any(axis=(0,2))
instead of the former triple-if loop over scanlines.
_compute_noise_arrays() now calls these two directly; get_noise() is kept
as a DeprecationWarning wrapper for external API compatibility.
7 new characterization tests in TestChannelNoise covering:
- return shape/dtype/dimensionality
- formula correctness (exact match via manual computation)
- bit-exact agreement with legacy get_noise on noaa14 fixture (bad_scans,
noise, av_noise, av_ict_noise for all three channels)
Regression: bit-exact on both quick-tier fixtures.
Phase 3D: Replace get_random/get_sys magic channel integer - Add _radiance_random_uncert(spec, ...) — spec.has_nonlinear replaces the if channel == 1 branch; data-driven, no magic integers - Add _radiance_sys_uncert(spec, ...) — same pattern; NaN propagates naturally through NumPy arithmetic - Deprecate get_random / get_sys with DeprecationWarning wrappers - 8 new tests in TestRadianceUncertHelpers (all green) Phase 3E: Vectorise the per-scanline for loop - Fix get_ict_uncert to accept scalar or array uICT (np.where replaces the scalar if np.isfinite guard) - Replace for i in range(len(CS_2)) with full NumPy broadcast: (N,) arrays col()-reshaped to (N,1) broadcast against CE (N,P) - bad_scan mask applied once at the end instead of via continue - ~7x speedup on 9000-scanline orbit (0.6s vs ~4.2s) - 57 tests pass; regression bit-exact
… helpers - Replace ~85-line output-assembly block in ir_uncertainty() with a clean helper function. - _uratio_to_uint8() consolidates the per-channel clip+encode pattern. - ir_uncertainty() now ends in a simple 8-line call to the helper. - 5 new unit tests (TestAssembleIRUncertaintyDataset); regression bit-exact.
- Extract _compute_vis_bad_scans: vectorised bad-scan detection replacing two per-scanline for loops - Extract _vis_channel_noise: per-channel helper eliminating the 7-tuple return from get_noise (deprecated to thin DeprecationWarning wrapper) - Extract _vis_sys_uncert: replaces get_sys(channel=int) magic number with explicit include_water_vapour=bool parameter - Extract _vis_random_uncert: drops 3 dead arguments (cal, year, jday) that were assigned but never used - Vectorise vis_uncertainty main loop: replaced for i in range(N) (~45 lines) with NumPy broadcasting; bad-scan mask applied once at end - 12 new unit tests across 5 test classes; regression bit-exact
- Extract _assemble_flags(ir_flags_1d, solar_contam_2d) helper - Replace per-scanline for loop with NumPy broadcasting: ir_flags[:, None] broadcast to (N, P) + bitwise OR for solar bit - 3 new unit tests (TestCombineFlagAssembly); regression bit-exact
The check `"3a" in ds["channels"]` tested float32 data values (never True), so nb_refl_channels was always 2 for noaa15/18 despite 6 channels being present. Result: ValueError when merging 3 IR channels into a 4-slot tail. Fix: derive the split from sizes — `ds.sizes["channel_name"] - irdata["random"].shape[-1]`. Adds TestCombineReflChannelCount (2 tests: 5-ch POD, 6-ch KLM). All 9 LHRR integration files now pass (noaa10, noaa11, noaa14, noaa15, noaa18).
0d73911 renamed the telemetry dataset variables and changed get_telemetry's arity, but the test doubles and synthetic fixtures were not updated, leaving 12 tests failing. Repair three of the four affected files, checking each expectation against the real production contract rather than adjusting fixtures until the current output passes. - Fake readers in test_reader.py returned the pre-0d73911 5-tuple (prt, ict, space, total_space, total_ict). Both POD and KLM readers now return (mean_prt_counts, full_space_counts, full_ict_counts), and reader.py unpacks them in that order, so the fakes follow. - test_read_to_dataset_... asserted the old prt_counts/ict_counts/ space_counts variable names and shapes. Updated to the names and shapes the reader actually builds, verified against the committed regression fixture noaa14_pod_d00322.input.nc. It also contained a live breakpoint(), which would hang any unattended run. - test_vis_uncertainty's synthetic dataset supplied vis_space_counts and total_vis_space_counts, which create_counts_dataset no longer produces, and gave counts the wrong dimensions. Rebuilt to the production schema. - Five failures shared one cause: since 0d73911 get_times() is @cache- decorated and no longer short-circuits when _times_as_np_datetime64 is already set, so it unconditionally re-reads and re-corrects the times from the file. Tests that install fixed timestamps must now also stop it doing that; preset_times() does so and documents why.
Until 0d73911 the dataset carried total_space_counts built from ir_channel_names, an IR-only 3-channel array, so indexing it [:, :, 0/1/2] meant 3.7/11/12 um. That commit replaced it with full_space_counts, which carries every channel, but the noise path kept indexing from 0. On a 5-channel AVHRR/2 dataset it therefore computed 3.7 um noise from channel 1 (visible) 11 um noise from channel 2 (visible) 12 um noise from channel 3 (3.7 um) and fed get_bad_space_counts the same wrong channels. Measured on noaa11_pod_d89214: 3.7 um noise 3.1x too small, 12 um 2.4x too large. Two conventions had grown up side by side -- get_vars and _channel_cal_arrays compensated with isel(channel_name=cal_index - 3) while the noise helpers did not -- so rather than add a third, introduce one accessor and use it everywhere: - ir_channels() with IR_CHANNEL_COUNT documents once that the IR channels are the last three in both the AVHRR/2 (1,2,3,4,5) and AVHRR/3 (1,2,3a,3b,4,5) layouts, and replaces the open-coded "- 3" at all five sites that had it. - build_ir_channel_data no longer takes total_space/total_ict. It already receives ds, and passing the arrays alongside is exactly what let them drift apart. _compute_bad_scans and _compute_noise_arrays are back to plain cal_index addressing, so their documented "shape (scanlines, counts_per_line, 3)" is true again. The new test states the property rather than the indexing: scrambling the reflective channels' space views must not move the IR noise. Corrective regression: both quick-tier baselines are regenerated. The change is confined to `random` and its shape matches the mechanism -- visible channels untouched, 3.7 um x3.09/x5.23, 11 um x1.009/x1.243, 12 um x0.412/x0.254 (noaa11/noaa14). The baselines were captured in 26d338c, after the bug was introduced, which is why they encoded it. test_all_uncertainties, whose expected values predate the refactor, fails before this change and passes after it.
…d to" This reverts commit 7a47bfe.
…ed to" This reverts commit 79cdf52.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ACUuGaXNjyzNqduE7fCNGM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
into code including a new module
which merges the two uncertainty outputs into a single xarray output.