Skip to content

add raw counts calibration type to Metop-SG A1 reader (vii_l1b_nc.py) #3373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tommyjasmin wants to merge 2 commits into
base: main
Choose a base branch
from
Open

add raw counts calibration type to Metop-SG A1 reader (vii_l1b_nc.py) #3373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1fac26b
add raw counts calibration type
tommyjasmin Apr 9, 2026
2c057bb
whoops. messed up indenting, and should catch KeyError
tommyjasmin Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 58 additions & 1 deletion satpy/etc/readers/vii_l1b_nc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ datasets:
file_key: data/measurement_data/vii_443
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
Copy link
Copy Markdown
Member

@djhoese djhoese Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this is inconsistent in Satpy, but I think counts should have units of "1". @simonrp84 @mraspaud @pnuu @gerritholl thoughts? Even the custom reader documentation says to do units: "count" as done here.

Copy link
Copy Markdown
Member

@gerritholl gerritholl Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the terminology of ISO 80000-1:2013 (the version I have a copy of, but I doubt it has radically changed), counts would be a quantity of dimension one (also referred to as dimensionless for historical reasons, but this is not strictly correct). ISO 80000-1 defines a unit of measurement as:

real scalar quantity, defined and adopted by convention, with which any other quantity of the same kind can
be compared to express the ratio of the second quantity to the first one as a number

Thinking about it, I don't think "count" meets this definition. Neither does 1. There is no quantity measured by "200 counts" that is 100 counts more than "100 counts". There is no convention that defines what 1 count is. And 1 is just a number.

Counts are recognised by UDUNITS, but that package has a rather liberal approach including anything that people use, and is not prescriptive on what is correct.

We can be pragmatic and use count, even if it is not strictly a unit.

We can be pragmatic and use 1, like we do for other quantities that have no unit, for example:

satpy/satpy/etc/readers/viirs_l2.yaml

Lines 110 to 113 in c1537e3

Angstrom_Exponent_Land_Ocean_Best_Estimate:
name: Angstrom_Exponent_Land_Ocean_Best_Estimate
long_name: Deep Blue/SOAR Angstrom exponent over land and ocean
units: "1"

Or we can be strict, and in that case I would set units empty or leave it out entirely.

When there is no unit "1" is not the worst to use, because ISO 80000-1 says we multiply a number with its unit to get a quantity, and multiplication with 1 is a no-op. But it doesn't really work, because counts — digital number — does not really express a quantity of anything in the first place.

BS EN ISO 80000-1-2013--[2017-03-23--10-09-20 AM].pdf

Copy link
Copy Markdown
Member

@gerritholl gerritholl Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In conclusion, I think counts — digital number — should not have a unit at all, because it is not calibrated and does not express a quantity.

Copy link
Copy Markdown
Member

@djhoese djhoese Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in other parts of Satpy we (or more likely past Dave's code) assume a non-empty units or at least a not-None units value.

Copy link
Copy Markdown
Contributor Author

@tommyjasmin tommyjasmin Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gerritholl - thank you for the commentary. I would push back on one thing, however.

You say a digital number does not express a quantity, based on ISO-80000-1, but I would argue it does.

A DN compares an incoming analog signal with a known reference, and scales that ratio. That's a quantity.

In fact I would even say DN is itself could be considered an appropriate answer here. But what is a DN in this world - it is a quantity where the physical units, volts/volts, cancel out with a result of 1.

I think the answer comes down to whether we prefer strict ISO compliance, or human readability. Counts is fairly well understood and prevents confusion with calibrated values.

One of y'all should just make an executive decision. 😁

Copy link
Copy Markdown
Member

@gerritholl gerritholl Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course the counts relate to a physical quantity, or the detector output couldn't be used to measure anything physical. But the number refers to a digital detector output, without a universal definition. The meaning of the difference between "1 count" and "2 counts", or even "-10 counts" (HIRS) depends on the instrument/detector. That is clearly not the counts I learned when covering CCDs in university, which are the number of photons falling onto the pixel multiplied by the quantum efficiency, and thus cannot be negative. That might just be a scaling for digital storage reasons, but it shows there is no one way to define a count. Microwave radiometers are again different entirely.

There's nothing about count in ISO 80000-1 explicitly. A web search for ISO "count" in a broader context yields mostly results about particle count in a context of ionising radiation (and even kilocount), something entirely different with the same word, and something with a direct physical definition. Those "counts" have unit 1: The unit of Becquerel is s⁻¹ (not count·s⁻¹).

If we must have a unit, I would argue for 1, because I don't think there is such a thing as a unit "count".

Copy link
Copy Markdown
Contributor Author

@tommyjasmin tommyjasmin Apr 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's go with "1". If I don't hear any further reasons for something else, I'll update the PR tomorrow, thanks everyone!

Copy link
Copy Markdown
Contributor Author

@tommyjasmin tommyjasmin Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If y'all want, once this PR is resolved, I'm happy to make a separate PR to update any remaining raw counts calibration references that are inconsistent.

Copy link
Copy Markdown
Member

@simonrp84 simonrp84 Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think it'd be nice to have an issue, rather than PR, where we can discuss what the correct unit for this should be.

Copy link
Copy Markdown
Contributor Author

@tommyjasmin tommyjasmin Apr 14, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think it'd be nice to have an issue, rather than PR, where we can discuss what the correct unit for this should be.

@simonrp84 - agree but to be clear, separate GitHub ticket/issue for adopting a convention system-wide. I still want to get this PR pushed through independently.

reflectance:
standard_name: toa_bidirectional_reflectance
units: "%"
Expand All @@ -68,6 +71,9 @@ datasets:
file_key: data/measurement_data/vii_555
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
reflectance:
standard_name: toa_bidirectional_reflectance
units: "%"
Expand All @@ -82,6 +88,9 @@ datasets:
file_key: data/measurement_data/vii_668
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
reflectance:
standard_name: toa_bidirectional_reflectance
units: "%"
Expand All @@ -96,6 +105,9 @@ datasets:
file_key: data/measurement_data/vii_752
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
reflectance:
standard_name: toa_bidirectional_reflectance
units: "%"
Expand All @@ -109,7 +121,7 @@ datasets:
file_type: nc_vii_l1b_rad
file_key: data/measurement_data/vii_763
coordinates: [lat_pixels, lon_pixels]
calibration: [reflectance, radiance]
calibration: [counts, reflectance, radiance]
chan_solar_index: 4
wavelength: [0.75695, 0.7627, 0.76845]

Expand All @@ -119,6 +131,9 @@ datasets:
file_key: data/measurement_data/vii_865
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
reflectance:
standard_name: toa_bidirectional_reflectance
units: "%"
Expand All @@ -133,6 +148,9 @@ datasets:
file_key: data/measurement_data/vii_914
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
reflectance:
standard_name: toa_bidirectional_reflectance
units: "%"
Expand All @@ -147,6 +165,9 @@ datasets:
file_key: data/measurement_data/vii_1240
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
reflectance:
standard_name: toa_bidirectional_reflectance
units: "%"
Expand All @@ -161,6 +182,9 @@ datasets:
file_key: data/measurement_data/vii_1375
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
reflectance:
standard_name: toa_bidirectional_reflectance
units: "%"
Expand All @@ -175,6 +199,9 @@ datasets:
file_key: data/measurement_data/vii_1630
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
reflectance:
standard_name: toa_bidirectional_reflectance
units: "%"
Expand All @@ -189,6 +216,9 @@ datasets:
file_key: data/measurement_data/vii_2250
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
reflectance:
standard_name: toa_bidirectional_reflectance
units: "%"
Expand All @@ -203,6 +233,9 @@ datasets:
file_key: data/measurement_data/vii_3740
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
brightness_temperature:
standard_name: toa_brightness_temperature
units: "K"
Expand All @@ -217,6 +250,9 @@ datasets:
file_key: data/measurement_data/vii_3959
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
brightness_temperature:
standard_name: toa_brightness_temperature
units: "K"
Expand All @@ -231,6 +267,9 @@ datasets:
file_key: data/measurement_data/vii_4050
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
brightness_temperature:
standard_name: toa_brightness_temperature
units: "K"
Expand All @@ -245,6 +284,9 @@ datasets:
file_key: data/measurement_data/vii_6725
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
brightness_temperature:
standard_name: toa_brightness_temperature
units: "K"
Expand All @@ -259,6 +301,9 @@ datasets:
file_key: data/measurement_data/vii_7325
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
brightness_temperature:
standard_name: toa_brightness_temperature
units: "K"
Expand All @@ -273,6 +318,9 @@ datasets:
file_key: data/measurement_data/vii_8540
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
brightness_temperature:
standard_name: toa_brightness_temperature
units: "K"
Expand All @@ -287,6 +335,9 @@ datasets:
file_key: data/measurement_data/vii_10690
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
brightness_temperature:
standard_name: toa_brightness_temperature
units: "K"
Expand All @@ -301,6 +352,9 @@ datasets:
file_key: data/measurement_data/vii_12020
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
brightness_temperature:
standard_name: toa_brightness_temperature
units: "K"
Expand All @@ -315,6 +369,9 @@ datasets:
file_key: data/measurement_data/vii_13345
coordinates: [lat_pixels, lon_pixels]
calibration:
counts:
standard_name: counts
units: "count"
brightness_temperature:
standard_name: toa_brightness_temperature
units: "K"
Expand Down
19 changes: 18 additions & 1 deletion satpy/readers/vii_l1b_nc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ def __init__(self, filename, filename_info, filetype_info, **kwargs):
self._bt_conversion_a = self["data/calibration_data/bt_conversion_a"].values
self._bt_conversion_b = self["data/calibration_data/bt_conversion_b"].values
self._channel_cw_thermal = self["data/calibration_data/channel_cw_thermal"].values
self._integrated_solar_irradiance = self["data/calibration_data/band_averaged_solar_irradiance"].values
# Test data has been seen for both variants below...
try:
self._integrated_solar_irradiance = self["data/calibration_data/band_averaged_solar_irradiance"].values
except KeyError:
self._integrated_solar_irradiance = self["data/calibration_data/Band_averaged_solar_irradiance"].values
Copy link
Copy Markdown
Member

@djhoese djhoese Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What files have the "B" and what files have the "b" naming?

Copy link
Copy Markdown
Contributor Author

@tommyjasmin tommyjasmin Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@djhoese - the test data I have, and am using for co-dev on the PyADDE server, have the cap-B:

(mtip) crud:MetOp tommyj$ ncdump -h W_XX-EUMETSAT-Darmstadt,SAT,SGA1-VII-1B-RAD_C_EUMT_20210510074835_G_D_20080223102004_20080223102104_T_B____.nc | grep Band
    	float Band_averaged_solar_irradiance(num_chan_solar) ;
    		Band_averaged_solar_irradiance:_FillValue = -999.f ;
    		string Band_averaged_solar_irradiance:long_name = "Band averaged solar irradiance" ;
    		string Band_averaged_solar_irradiance:units = "W/(m^2 µm)" ;
    		Band_averaged_solar_irradiance:valid_min = 0.f ;
    		Band_averaged_solar_irradiance:valid_max = 2400.f ;

I am unsure of the origin for this test data, but can find out if needed.

For some reason, back in 2024, the cap "B" was changed to a lowercase "b". See:

1c0ce37

I am under the assumption there must be test data with both variants in the wild, and added the try/except to support that.

Copy link
Copy Markdown
Member

@djhoese djhoese Apr 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I guess my main concern is whether this should be supported if it is from old files or from a different processing chain. If it is possible your test data is old and it is reasonable for you to get newer test data then it'd be great to have simpler code here. If this try/except stays in then maybe a comment could be added to the cap-B case to state it is for older test data or whatever is actually the case.

# Computes the angle factor for reflectance calibration as inverse of cosine of solar zenith angle
# (the values in the product file are on tie points and in degrees,
# therefore interpolation and conversion to radians are required)
Expand Down Expand Up @@ -83,6 +87,19 @@ def _perform_calibration(self, variable: xr.DataArray, dataset_info: dict) -> xr
calibrated_variable.attrs = variable.attrs
elif calibration_name == "radiance":
calibrated_variable = variable
elif calibration_name == "counts":
# xarray automatically applies scale_factor and add_offset when reading the netCDF.
# To get raw counts, reverse this process using the original parameters.
Comment on lines +91 to +92
Copy link
Copy Markdown
Member

@djhoese djhoese Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could ask xarray not to do this and then have the radiance calculation do the scaling.

Copy link
Copy Markdown
Contributor Author

@tommyjasmin tommyjasmin Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hhmmm... not sure what is most appropriate at the moment. Need to have a think on this one.

Copy link
Copy Markdown
Contributor

@jon4than jon4than Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's pretty easy to turn off in ViiNCBaseFileHandler:
super().__init__(filename, filename_info, filetype_info, auto_maskandscale=True)

Turning off auto_maskandscale does mean that a few other netCDF variables need to be manually unpacked. Neither Tommy or I were sure which approach would be preferable for Satpy. We ended up opting for the least invasive approach for now.

Copy link
Copy Markdown
Member

@djhoese djhoese Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I'm not completely against the way it is implemented, but others may feel differently.

scale_factor = variable.encoding.get("scale_factor", variable.attrs.get("scale_factor", 1.0))
add_offset = variable.encoding.get("add_offset", variable.attrs.get("add_offset", 0.0))

calibrated_variable = (variable - add_offset) / scale_factor

# Cast back to the original integer datatype (e.g., uint16) for strict counts
original_dtype = variable.encoding.get("dtype", variable.dtype)
calibrated_variable = calibrated_variable.astype(original_dtype)

calibrated_variable.attrs = variable.attrs
else:
raise ValueError("Unknown calibration %s for dataset %s" % (calibration_name, dataset_info["name"]))

Expand Down
Loading