Work around a Cython comparison miscompilation in GroupBond._change_bond (#1029)

`arc/molecule/group_test.py::TestGroupBond::{test_apply_action_increment_bond,test_apply_action_decrement_bond}`
fail with

```
ActionError: Unable to update Bond due to CHANGE_BOND action: Invalid resulting order "[2]"
```

for an order the guard accepts — it raises only when a value is below 0
or above 4.

## Cause

**Cython 3.3.0 miscompiles a comparison in the element expression of a
list comprehension.** Minimal reproducer, built with `language_level:
3`:

```python
def element_form(list vals):
    return [v > 4 for v in vals]
```

| `vals` | Python | compiled |
|---|---|---|
| `[2]` | `[False]` | **`[True]`** |
| `[5]` | `[True]` | **`[False]`** |
| `[100]` | `[True]` | **`[False]`** |
| `[0]` | `[False]` | **`[True]`** |
| `[4]` | `[False]` | `[False]` |
| `[-1]` | `[True]` | `[True]` |

Three forms of the same comparison are correct: a generator expression,
an explicit loop, and a comprehension's **filter** clause. Arithmetic in
an element expression is also correct, verified separately, so
`new_order` itself was never wrong — only the guard reading it.

The guard used the broken form:

```python
new_order = [value + order for value in self.order]
if any([value < 0 or value > 4 for value in new_order]):
```

`any()` is not implicated; it reported a list that had already been
computed incorrectly.

## Change

The guard collects the out-of-range entries with a filter clause, which
is measured correct for every value tested, and the error names them:

```
Invalid resulting order "[5]". The orders outside the range [0, 4] are [5] of types [int],
reached from the orders [4] of types [int] by an increment of 1 of type int.
```

The previous message printed only the resulting list, so it could not
distinguish an `int` from another type that formats identically, did not
say which entry tripped the guard, and did not report the increment.
That is most of why this took a day to characterise.

## It is not intermittent

It appeared to be, because it turned up on unrelated branches and the
same commit both passed and failed. What varied was the Cython version
each run resolved: `environment.yml` requires `cython >=3.1` with no
upper bound, so runs that picked up 3.3.0 failed and earlier resolves
passed. In an environment built from `environment.yml`, it reproduces
**serially, in 1.3 seconds**:

```
pytest arc/molecule/group_test.py   ->  2 failed, 58 passed in 1.26s
```

Isolating the two halves of this change confirms which one matters:
renaming the loop variables while keeping `any([...])` still fails, and
replacing the comprehension while keeping the names passes.

## Two commits

**Work around the miscompilation at the failing call site.** The guard
collects the out-of-range entries with a filter clause, which is
measured correct for every value tested.

**Bound the compiler.** `environment.yml` required `cython >=3.1` with
no upper bound, so the compiler that builds `arc.molecule` was whatever
conda-forge had published most recently, and it changed between two runs
of the same commit. It is now `>=3.1,<3.3`. Cython 3.2.4 returns the
correct result for all eight values above; the bound is below 3.3 rather
than below 3.4 because 3.4 would still admit 3.3.0.

The second commit is the one that matters more. The first fixes the call
site whose failure is visible; the same construct appears elsewhere in
the Cythonised modules, and it is wrong in both directions — a value
above the range compiling to `False` means an invalid input is
**accepted** rather than rejected, which no test would catch.

Author: kfir4444

arc/molecule/group.py

@@ -1039,11 +1039,26 @@ def _change_bond(self, order):
         Update the bond group as a result of applying a CHANGE_BOND action,
         where `order` specifies whether the bond is incremented or decremented
         in bond order. `order` is normally 1 or -1, but can be any value
+
+        Each comprehension below binds its own loop variable. This function is declared ``cpdef`` in
+        ``group.pxd``, and Cython inlines a comprehension inside a ``cpdef`` function into the
+        function's own scope rather than giving it a scope of its own, so a loop variable reused
+        across two comprehensions here is one variable rather than two.
         """
         new_order = [value + order for value in self.order]
-        if any([value < 0 or value > 4 for value in new_order]):
+        out_of_range = [new_value for new_value in new_order if new_value < 0 or new_value > 4]
+        if out_of_range:
             raise ActionError('Unable to update Bond due to CHANGE_BOND action: '
-                              'Invalid resulting order "{0}".'.format(new_order))
+                              'Invalid resulting order "{0}". The orders outside the range [0, 4] are [{1}] '
+                              'of types [{2}], reached from the orders [{3}] of types [{4}] by an increment '
+                              'of {5!r} of type {6}.'.format(
+                                  new_order,
+                                  ', '.join([repr(bad_value) for bad_value in out_of_range]),
+                                  ', '.join([type(bad_value).__name__ for bad_value in out_of_range]),
+                                  ', '.join([repr(old_value) for old_value in self.order]),
+                                  ', '.join([type(old_value).__name__ for old_value in self.order]),
+                                  order,
+                                  type(order).__name__))
         # Change any modified benzene orders to the appropriate stable order
         new_order = set(new_order)
         if 0.5 in new_order:

environment.yml

@@ -22,7 +22,7 @@ dependencies:
   - conda-forge::cairocffi
   - conda-forge::cmake
   - conda-forge::coverage
-  - conda-forge::cython >=3.1
+  - conda-forge::cython >=3.1,<3.3
   - conda-forge::ffmpeg
   - conda-forge::gprof2dot
   - conda-forge::graphviz