Repository: ioos/compliance-checker (python)

Issue:
`check_calendar` is slow for large files
**Version of compliance checker running: v5.1.1 (also tried v5.3.0)**

**Describe the checker this affects:**

[cf_1_6.py](https://github.com/ioos/compliance-checker/blob/main/compliance_checker/cf/cf_1_6.py)
[check_calendar](https://github.com/ioos/compliance-checker/blob/1110ea52a24db3502d6be5b6f8cc86da455544b0/compliance_checker/cf/cf_1_6.py#L1895)

**Attach a minimal CDL or NetCDF file which is able to reproduce the issue**

[any file available here](https://thredds.aodn.org.au/thredds/catalog/IMOS/SRS/ALTIMETRY/calibration_validation/geodeticGNSS/BASS/catalog.html)

```
dimensions:
	TIME = UNLIMITED ; // (5007551 currently)
variables:
	double TIME(TIME) ;
		TIME:units = "days since 1950-01-01 00:00:00 UTC" ;
		TIME:standard_name = "time" ;
		TIME:long_name = "time" ;
		TIME:axis = "T" ;
		TIME:valid_max = 36525. ;
		TIME:calendar = "gregorian" ;
		TIME:comment = "Time is provided at the observation frequency of 2 hz" ;
		TIME:valid_min = 0. ;

```
**To Reproduce:**

We are running the compliance checker with the following configuration:

```
checks: [ "cf:1.6", "imos:1.4"]
skip_checks: ["check_convention_globals:M",
              "check_vertical_variable:A",
              "check_vertical_variable_reference_datum:A",
              "check_quality_control_variable_standard_name:A"
  ]
```

**Describe the issue below:**

When running the compliance checker (v5.1.1 or v5.3.0) with a file that is 1.2mb, it takes 22 seconds to complete. If we add the following skip: `"check_calendar:A"` it only takes 1 second to complete.  

It takes a long time to complete when running the compliance checker against multiple files of > 250mb in size.  I just wanted to raise this issue. For now, we can skip this check as we are applying a [plugin](https://github.com/aodn/cc-plugin-imos) with a similar check as well. 

Using snakeviz, I identified that the following checks were causing the delay for a 270mb file:

![Image](https://github.com/user-attachments/assets/012224da-796f-47cd-aab9-e5f5d5fb47f4)

 I stopped it midway through after waiting 330 seconds. A previous time, it was taking hours when checking multiple larger files (>250mb). 







Pull request:
Avoid comparing cftime objects due to poor performace
Closes #1211

@benjwadams do you mind taking a look at this one? While it passes all the tests, dealing with dates is always quite complex and error prone.

@mphemming thanks for the detailed issue in #1211. Can you test this branch against your data and see if it helps?

xref.: https://github.com/Unidata/cftime/issues/365

Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 9540f74..25ec4bf 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -31,7 +31,7 @@ repos:
 
 
 - repo: https://github.com/astral-sh/ruff-pre-commit
-  rev: v0.11.12
+  rev: v0.12.0
   hooks:
     - id: ruff
       args: ["--fix", "--show-fixes"]
diff --git a/compliance_checker/cf/cf_1_11.py b/compliance_checker/cf/cf_1_11.py
index a472c12..eb831b8 100644
--- a/compliance_checker/cf/cf_1_11.py
+++ b/compliance_checker/cf/cf_1_11.py
@@ -105,3 +105,19 @@ class CF1_11Check(CF1_10Check):
                 time_units_metadata_ctx.score += 1
 
         return [time_units_metadata_ctx.to_result()]
+
+    def check_single_cf_role(self, ds):
+        test_ctx = self.get_test_ctx(
+            BaseCheck.HIGH,
+            self.section_titles["9.5"],
+        )
+        cf_role_var_names = [
+            var.name
+            for var in (ds.get_variables_by_attributes(cf_role=lambda x: x is not None))
+        ]
+        test_ctx.assert_true(
+            len(cf_role_var_names) < 2,
+            "There may only be one variable containing the cf_role attribute. "
+            f"Currently the following variables have cf_role attributes: {cf_role_var_names}",
+        )
+        return test_ctx.to_result()
diff --git a/compliance_checker/cf/cf_1_6.py b/compliance_checker/cf/cf_1_6.py
index 80972ff..b8d2f0b 100644
--- a/compliance_checker/cf/cf_1_6.py
+++ b/compliance_checker/cf/cf_1_6.py
@@ -1954,36 +1954,38 @@ class CF1_6Check(CFNCCheck):
             Check that the time variable does not cross the date
             1582-10-15 when standard or gregorian calendars are used
             """
+            # Short-circuit if using months/years.
+            if any(unit in time_var.units for unit in ("months", "years")):
+                return Result(
+                    BaseCheck.LOW,
+                    False,
+                    self.section_titles["4.4"],
+                    [
+                        "Miscellaneous failure when attempting to calculate crossover, possible malformed date",
+                    ],
+                )
+            time_values = time_var[:]
+
             # IMPLEMENTATION CONFORMANCE 4.4.1 RECOMMENDED 2/2
             # Only get non-nan/FillValue times, as these are the only things
             # that make sense for conversion.  Furthermore, non-null checks
             # should be made for time coordinate variables anyways, so errors
             # should be caught where implemented there
             crossover_date = cftime.DatetimeGregorian(1582, 10, 15)
+            crossover_date_value = cftime.date2num(
+                crossover_date,
+                time_var.units,
+                calendar=time_var.calendar,
+                has_year_zero=True,
+            )
             # has_year_zero set to true in order to just check crossover,
             # actual year less than or equal to zero check handled elsewhere
             # when standard/Gregorian, or Julian calendars used.
 
-            # WARNING: might fail here if months_since are used and suppress
-            #          usual warning
-            try:
-                times = cftime.num2date(
-                    time_var[:].compressed(),
-                    time_var.units,
-                    has_year_zero=True,
-                )
-            except ValueError:
-                return Result(
-                    BaseCheck.LOW,
-                    False,
-                    self.section_titles["4.4"],
-                    [
-                        "Miscellaneous failure when attempting to calculate crossover, possible malformed date",
-                    ],
-                )
-
-            crossover_1582 = np.any(times < crossover_date) and np.any(
-                times >= crossover_date,
+            # Comparing cftime objects is awfully slow. Converting them toordinal makes this a bit faster.
+            # https://github.com/ioos/compliance-checker/issues/1211
+            crossover_1582 = np.any(time_values < crossover_date_value) and np.any(
+                time_values >= crossover_date_value,
             )
             if not crossover_1582:
                 reasoning = (
@@ -3722,11 +3724,6 @@ class CF1_6Check(CFNCCheck):
                 "".format(cf_role, ", ".join(valid_roles)),
             )
         if variable_count > 0:
-            m = (
-                "§9.5 The only acceptable values of cf_role for Discrete Geometry CF"
-                + " data sets are timeseries_id, profile_id, and trajectory_id"
-            )
-            valid_cf_role.assert_true(variable_count < 3, m)
             return valid_cf_role.to_result()
 
     def check_variable_features(self, ds):
diff --git a/compliance_checker/cf/cf_1_8.py b/compliance_checker/cf/cf_1_8.py
index cda2fd3..24cf0a9 100644
--- a/compliance_checker/cf/cf_1_8.py
+++ b/compliance_checker/cf/cf_1_8.py
@@ -132,8 +132,14 @@ class CF1_8Check(CF1_7Check):
         # Using the group B dimension to match a variable from group A.
         # This is logically wrong: NetCDF allows it, but the dimensions are different objects.
         # This may not throw during creation, so we simulate an access/mismatch
+        # grab the first group's time‐dim
+        grp_list = list(ds.groups.keys())
+        first_dim = ds.groups[grp_list[0]].dimensions["time"]
+
+        # check that every other group's time‐dim is the very same object
+
         invalid_same_named_dimension_across_groups.assert_true(
-            ds.groups["A"].dimensions["time"] is ds.groups["B"].dimensions["time"],
+            all(ds.groups[g].dimensions["time"] is first_dim for g in grp_list[1:]),
             "Dimensions with the same name must be the same object (ID).",
         )
 


Test patch:
diff --git a/compliance_checker/tests/test_cf.py b/compliance_checker/tests/test_cf.py
index a6fc3cb..be575ff 100644
--- a/compliance_checker/tests/test_cf.py
+++ b/compliance_checker/tests/test_cf.py
@@ -1283,6 +1283,17 @@ class TestCF1_6(BaseTestCase):
         assert (scored, out_of) == (1, 2)
         # TEST CONFORMANCE 4.4 REQUIRED 2/2, RECOMMENDED 1, 2/2
         dataset = MockTimeSeries()
+        # CF reference times should contain "since"
+        # The below are all forms are all valid UDUNITS but aren't
+        # supported in CF
+        time_fmt_string = "seconds {} 0-1-1 23:00"
+        for noncompliant_preposition in ("@", "after", "from", "ref"):
+            invalid_ref_time = time_fmt_string.format(noncompliant_preposition)
+            dataset.variables["time"].units = invalid_ref_time
+            results = self.cf.check_time_coordinate(dataset)
+            scored, out_of, messages = get_results(results)
+            assert scored < out_of
+            assert "time does not have correct time units" in messages
         # NB: >= 60 seconds is nonstandard, but isn't actually a CF requirement
         # until CF 1.9 onwards
         dataset.variables["time"].units = "months since 0-1-1 23:00:60"
@@ -3531,6 +3542,23 @@ class TestCF1_11(BaseTestCase):
         scored, out_of, _ = get_results(results)
         assert scored == 1 and out_of == 2
 
+    def test_single_cf_role(self):
+        ds = MockTimeSeries()
+        ds.createDimension("ts_no")
+        ts = ds.createVariable("ts", "i4", ("ts_no",))
+        ts.cf_role = "timeseries_id"
+        result = self.cf.check_single_cf_role(ds)
+        assert result.value[0] == result.value[1]
+        ts2 = ds.createVariable("ts2", "i4", ("ts_no",))
+        ts2.cf_role = "timeseries_id"
+        result = self.cf.check_single_cf_role(ds)
+        assert result.value[0] < result.value[1]
+        assert (
+            "There may only be one variable containing the cf_role attribute. "
+            "Currently the following variables have cf_role attributes: ['ts', 'ts2']"
+            in result.msgs
+        )
+
 
 class TestCFUtil(BaseTestCase):
     """