Context
While exploring the codebase, I noticed that the module docstring of physical_field.py already explicitly mentions wind speed as an example use case:
"...but also of other physical fields, such as wind speed, which is a function of both meridional and zonal wind components."
This led me to implement calculate_wind_speed(u, v) following the same pattern as the existing calculate_toa_radiation function, along with tests. The branch is here: https://github.com/RajdeepKushwaha5/mllam-data-prep/compare/main...add-calculate-wind-speed
Question for the community
@joeloskarsson raised a valid point: in neural-lam, u and v are typically kept separate as forcing features since they encode direction. I understand that concern.
However, I can think of cases where wind speed magnitude as a derived forcing could be useful:
- A user's source dataset only has u and v, but their target model expects wind speed as an input feature
- Simpler or non-neural model architectures that use wind speed directly
- Diagnostic or post-processing use cases
Two possible outcomes
- Keep it — wind speed is a valid derived physical field, and the docstring already signals this intent
- Drop it + update the docstring — remove wind speed from the docstring example to avoid confusion for future contributors
Either way, the docstring should match what the module actually supports. Happy to close this PR or update the docstring instead — just wanted to raise it properly since the existing docstring pointed in this direction.
Context
While exploring the codebase, I noticed that the module docstring of
physical_field.pyalready explicitly mentions wind speed as an example use case:This led me to implement
calculate_wind_speed(u, v)following the same pattern as the existingcalculate_toa_radiationfunction, along with tests. The branch is here: https://github.com/RajdeepKushwaha5/mllam-data-prep/compare/main...add-calculate-wind-speedQuestion for the community
@joeloskarsson raised a valid point: in neural-lam, u and v are typically kept separate as forcing features since they encode direction. I understand that concern.
However, I can think of cases where wind speed magnitude as a derived forcing could be useful:
Two possible outcomes
Either way, the docstring should match what the module actually supports. Happy to close this PR or update the docstring instead — just wanted to raise it properly since the existing docstring pointed in this direction.