pygac icon indicating copy to clipboard operation
pygac copied to clipboard

Calculation of pixel lat/lon does not work for GAC files

Open oembury opened this issue 9 months ago • 6 comments

Calling _compute_missing_lonlat for GAC files produces very incorrect results. This appears to be a bug in pytroll/pyorbital#184 and can be fixed with pytroll/pyorbital#185 However, it looks pygac always uses the avhrr_gac instrument definition, so fixing the "gac" bug may break LAC processing...

Would it be better to move the current _compute_missing_lonlat to gac_reader.py and create another version in lac_reader.py using pyorbital.geoloc_instrument_definitions.avhrr ?

oembury avatar Mar 18 '25 15:03 oembury

Thanks for the report! @mraspaud Could this explain (parts of) the navigation problems you are working on?

sfinkens avatar Mar 19 '25 09:03 sfinkens

@oembury could you provide an example of what "very incorrect results" look like? afaik, we have been using the pyorbital code for processing without major problems so far, so I'm a bit surprised.

mraspaud avatar Mar 19 '25 15:03 mraspaud

See attached for NSS.GHRR.NC.D81242.S0756.E0950.B0095859.GC (blue dots are the output from _get_lonlat_from_file()) Sorry, the "very incorrect results" was a bit excessive - I was originally looking at a file with a lot more timing / gap issues

pygac will normally attempt to interpolate from the existing lat/lon coordinates and will only call pyorbital if the clock adjusments mean it needs lat/lons from a data gap. However, if you call _get_lonlat_from_file directly to calculate all the coordinates it's clear that it's only returning one side of the swath

Note - you can probably ignore the outliers in the middle get_lonlat() plot. I'm skipping the time / scanline corrections at the moment.

Image

oembury avatar Mar 19 '25 16:03 oembury

@oembury thanks a lot for the explanation. I had a quick look, and I can indeed reproduce the issue.

It seems to me that one solution is to replace this line: https://github.com/pytroll/pygac/blob/main/pygac/gac_reader.py#L45

with something in the style of

lonlat_sample_points = GAC_LONLAT_SAMPLE_POINTS * 5

so as to keep the lac data working. But at the same time, the avhrr_gac name in pyorbital is indeed confusing as it's used for lac data too, and it samples over 2048 points...

mraspaud avatar Mar 19 '25 17:03 mraspaud

That should get the right swath width (assuming the offset term is also included). Though I think the code would be rather confusing to follow - especially if there is also a fractional pixel offset #88 to account for.

pyorbital already contains an avhrr instrument definition which looks like it is supposed to be the full resolution (LAC / FRAC) version of avhrr_gac. This would have the advantage of providing the correct default frequency (1/6s) rather than gac 1/2s. I guess the avhrr definition would need both "number of scans" and "scan times" options like the gac version does, but that should be a trivial change?

oembury avatar Mar 19 '25 18:03 oembury

I agree that we need to make things easier to understand, but we need to make sure backwards compatibility between pyorbital and pygac is preserved (latest releases of pygac were working fine I think)

Suggested plan of action:

  1. Add new functions in pyorbital, eg avhrr_from_times and avhrr_gac_from_times that include the fixes you provided, and add a deprecation warning to avhrr_gac since it's ambiguous, and make a pyorbital release
  2. Make pygac use the new pyorbital version if available.

I'll start with reviewing your pyorbital PR.

mraspaud avatar Mar 20 '25 08:03 mraspaud

@oembury the pyorbital changes have now been merged and released (v1.10.0)

mraspaud avatar Apr 10 '25 09:04 mraspaud