Missing `EARTH_RADIUS` in mapping satellite's geodetic coordinates to cartesian coordinates while doing parallax correction
Open
@gerritholl is already working on this.
Since Sep 4, 2022.
- Dominant language
- Python
- Stars
- 1.2k
- Forks
- 335
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 10
Description
Describe the bug
Mapping geodetic coordinate to cartesian coordinates should use radial distance.
For a satellite, its radial distance is EARTH_RADIUS + satellite_altitude instead of satellite_altitude.
To Reproduce
Copy codes from
I have codes below.
import numpy as np
from satpy.modifiers.parallax import EARTH_RADIUS
from satpy.utils import lonlat2xyz
sat_lon = 0
sat_lat = 90.
sat_alt = EARTH_RADIUS * 1e3 # satellite altitude in meters, equal to Earth radius
lon = 0 # cloud longitude
lat = 90 # cloud latitude
sat_xyz = np.hstack(lonlat2xyz(sat_lon, sat_lat)) * sat_alt
cth_xyz = np.stack(lonlat2xyz(lon, lat), axis=-1) * EARTH_RADIUS * 1e3 # km → m
assert not np.all(sat_xyz == cth_xyz) # sat_xyz must not equals cth_xyz in this scenario
Expected behavior
Passed assert not np.all(sat_xyz == cth_xyz).
Actual results
AssertionError
Screenshots
Omitted.
Environment Info:
Omitted.
Additional context
# I think the fix must be like
sat_xyz = np.hstack(lonlat2xyz(sat_lon, sat_lat)) * (sat_alt + EARTH_RADIUS * 1e3) # EARTH_RADIUS km → m
Well, the fix is simple, should I make a PR/MR?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.