Add maps to TUV-x Python constructor#680
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the TUV-x Python API by requiring grid, profile, and radiator maps to be passed to the constructor, shifting from a config-only approach to a more flexible host-controlled model. The API now requires explicit solar zenith angle and Earth-Sun distance parameters for each run, removing the previous batch-processing mode.
Key changes:
- TUV-x constructor now requires GridMap, ProfileMap, and RadiatorMap instances alongside config
- Run method signature changed to accept solar zenith angle and Earth-Sun distance parameters
- Removed config-only mode, GetNumberOfSzaSteps(), RunFromConfig(), and related infrastructure
- Output arrays changed from 3D (sza_steps, layers, rates) to 2D (layers, rates) format
Reviewed Changes
Copilot reviewed 14 out of 17 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
src/tuvx/tuvx.cpp |
Updated Create and Run methods to accept maps; removed config-only mode logic and helper methods |
src/tuvx/tuvx_c_interface.cpp |
Added dose_rates parameter to RunTuvx function signature |
include/musica/tuvx/tuvx.hpp |
Updated class interface to remove config-only methods and add map parameters |
include/musica/tuvx/tuvx_c_interface.hpp |
Added CreateTuvxFromConfigString with maps; updated function signatures |
src/tuvx/interface.F90 |
Moved and updated internal_create_tuvx_from_config_string to accept maps; removed RunFromConfig and GetNumberOfSzaSteps |
fortran/tuvx/tuvx.F90 |
Updated run_tuvx_c interface to include dose_rates parameter |
python/bindings/tuvx/tuvx.cpp |
Updated Python bindings to pass maps and handle 2D output arrays |
python/musica/tuvx/tuvx.py |
Updated constructor and run method signatures to require maps and runtime parameters |
python/musica/__init__.py |
Added RadiatorMap and Radiator to exports |
python/test/integration/test_tuvx.py |
Added helper functions for creating maps; updated tests for new API |
src/test/unit/tuvx/tuvx_run_from_config.cpp |
Updated test calls to pass nullptr for dose_rates |
src/test/data/tuvx/full_from_host/* |
Added new test data files with NetCDF cross sections and configuration |
configs/tuvx/tuv_5_4.json |
Removed grid and profile definitions (now provided by host) |
Comments suppressed due to low confidence (1)
python/test/integration/test_tuvx.py:227
- The docstring parameter
dose_rates: np.ndarrayis documented but the method signature forget_dose_ratedoesn't show this parameter. This appears to be inconsistent with the actual signature visible in the diff context. Based on other similar methods in the codebase, the dose_rates parameter should be explicitly shown in the method signature documentation.
radiator_map["hot air balloons"] = hot_air_balloons
return radiator_map
def test_tuvx_version():
version = musica.tuvx.version
assert version is not None
assert isinstance(version, str)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "type": "base", | ||
| "netcdf files": [ | ||
| { | ||
| "file path": "test/data/tuvx/fixed/O2_cross_section.nc" |
There was a problem hiding this comment.
The configuration references "test/data/tuvx/fixed/O2_cross_section.nc" but there's an "O2_cross_section.nc" file in the same "full_from_host" directory. This should likely be "test/data/tuvx/full_from_host/O2_cross_section.nc" to reference the local file.
| "file path": "test/data/tuvx/fixed/O2_cross_section.nc" | |
| "file path": "test/data/tuvx/full_from_host/O2_cross_section.nc" |
K20shores
left a comment
K20shores
left a comment
There was a problem hiding this comment.
Seems fine to me, and I think some of the copilot comments for file paths seem like they could be accepted
This PR adds the ability to pass grid, profile, and radiator maps to the TUV-x constructor in Python.
closes #600