Skip to content

Conversation

@sunt05
Copy link

@sunt05 sunt05 commented Dec 23, 2025

Summary

Improves the vendored f90wrap code generation to ensure Qt/QGIS compatibility by hardening the module path handling and removing conflicting SIGINT handler modifications.

Changes

  • Vendor f2py_f90wrap.py with Qt-safe error handling
  • Remove SIGINT handler conflicts with Qt signal handlers
  • Add comprehensive Qt integration tests for Windows QGIS CI
  • Improve f90wrap path robustness in module loading

Quick Reproduction

from supy import SUEWSSimulation

sim = SUEWSSimulation.from_sample_data()

# Set z < zdm_in via config to trigger crash
sim.update_config({
    "sites": {
        "properties": {
            "z": {"value": 5.0},
            "zdm_in": {"value": 15.0}
        }
    }
})

# This triggers the error
sim.run(end_date=sim.forcing.index[287])

Fixes #1035

@sunt05 sunt05 marked this pull request as ready for review December 26, 2025 12:21
@sunt05 sunt05 force-pushed the sunt05/gh1035-check branch 2 times, most recently from 8aebf0f to 69d1aa1 Compare January 7, 2026 21:21
@sunt05 sunt05 force-pushed the sunt05/gh1035-check branch from 9b4a960 to 0adaaed Compare January 13, 2026 09:49
@sunt05
Copy link
Author

sunt05 commented Jan 13, 2026

Additional Fix: Early Return for Fatal Errors

Added RETURN after ErrorHint(32) in cal_Stab subroutine (suews_phys_atmmoiststab.f95) to prevent LOG(negative) crash.

Before: ErrorHint set the error state, but execution continued to LOG(zzd/z0m) which crashed on negative values.

After: Subroutine returns immediately after setting error state, allowing Python to catch SUEWSKernelError.

This completes the fix for #1035 - the SIGINT handler removal ensures Qt compatibility, and this change ensures fatal errors exit cleanly before hitting undefined math operations.

sunt05 and others added 25 commits January 13, 2026 15:30
- Remove SIGINT handler modification that conflicts with Qt signal handlers
- Add vendored f2py_f90wrap.py with Qt-safe error handling
- Add headless Qt tests for error handling validation
- Add GitHub Actions workflow for Windows QGIS CI testing

The f90wrap SIGINT handler (`PyOS_setsig`) conflicts with Qt's signal
handlers on Windows, causing QGIS to crash when SUEWS errors occur.
This fix removes the SIGINT modification while preserving the
setjmp/longjmp error handling mechanism.

Trade-off: Ctrl+C during Fortran calls must wait for call to return
(~ms), but Qt/QGIS compatibility is gained.
For pull_request and workflow_dispatch triggers, build supy from source
to validate fixes before TestPyPI release. Only use TestPyPI for
workflow_run (after nightly builds).
- Add test_pyqgis_error_handling.py for testing inside actual QGIS context
- Merge QGIS tests into build-publish workflow (tests actual UMEP wheel)
- Remove standalone test-qgis-windows.yml (superseded by integrated job)
- Remove redundant Qt-only tests (test_qt_*.py)
- Keep only test_pyqgis_error_handling.py (most comprehensive)
- Add --ignore for test/qgis in cibuildwheel (run separately)
- Simplify test_qgis job to single test step

The PyQGIS test uses actual QgsApplication context, making it
more realistic than standalone Qt tests for validating GH-1035.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
OSGeo4W doesn't expose pip directly in PATH. Use python -m pip
instead to install the UMEP wheel.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
OSGeo4W Python doesn't include pip by default. Use ensurepip to
bootstrap pip before installing the UMEP wheel.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Set PYTHONPATH to include QGIS Python bindings (OSGeo4W v2 has no
  qgis_env.bat)
- Add qgis_test_only workflow input for fast iteration using TestPyPI
  wheels instead of building from source
- Skip builds when qgis_test_only is enabled

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add standalone test script that initialises QGIS in headless mode and
verifies SUEWS error handling works correctly without crashing.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Separate workflow that uses TestPyPI wheels directly - no building.
Trigger via Actions > Test QGIS Windows Compatibility > Run workflow.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use Qgis.version() instead of QgsApplication.version() - the latter
doesn't exist in QGIS 3.x API.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Set QT_QPA_PLATFORM=offscreen at shell level before running Python,
ensuring Qt sees the variable before any initialization. More defensive
than relying solely on Python os.environ setting.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use quoted set syntax for proper empty string handling in Windows CMD.
The quoted form `set "VAR=value"` correctly handles empty values.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add early exit in SUEWS_cal_multitsteps when supy_error_flag is set
- Fix SUEWSKernelError being caught as RuntimeError subclass
- Re-raise kernel errors without wrapping in run_supy_ser
- Scan all timesteps for errors (handles early exit case)

Previously, fatal errors would repeat for every remaining timestep
(288 times for a 1-day simulation). Now exits after first error.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove commented-out debugging code:
- Removed commented print statement for temp directory path
- Removed commented pdb.set_trace() in pack_var function
- Removed extensive debug comparison code in pack_grid_dict
- Removed redundant else:pass clause

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The UMEP wheel already has numpy>=1.22,<2.0 baked into its metadata
from the build action, so explicit constraint is unnecessary.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The main test file uses Qgis.version() which works on all platforms.
The macOS variant was created during debugging with the older
Qgis.QGIS_VERSION attribute and is no longer needed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The PyQGIS error handling test is really about UMEP wheel compatibility,
not QGIS itself. Consolidate with existing UMEP tests for cleaner
organisation.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove test_imports.py: All imports already tested in other modules
- Merge test_postprocessor.py into test_processor.py: Single test for
  config output path fits better with processor tests
- Update __init__.py to reflect consolidated structure

Reduces test files from 7 to 5 and removes ~90 lines of redundancy.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Merge test_environment.py, test_preprocessor.py, and test_processor.py
into test_umep_api.py. All three test files share the same skip logic
(Windows + Python 3.12 only) and test UMEP API compatibility.

Final structure (4 files, 474 lines):
- conftest.py: Shared skip logic
- test_umep_api.py: All UMEP API tests (skipped unless Windows + Py3.12)
- test_pyqgis_error_handling.py: PyQGIS error handling (runs in CI)

Reduces from 660 to 474 lines (-186 lines).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Each test module now has its own pytestmark for skip logic.
Removes conftest.py dependency and simplifies structure.

Final structure (3 files, 449 lines):
- __init__.py: Package docstring (5 lines)
- test_umep_api.py: All UMEP API tests (331 lines)
- test_pyqgis_error_handling.py: PyQGIS error handling (113 lines)

Total reduction: 660 → 449 lines (-32%)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
OSGeo4W environment doesn't have pytest installed. The script runs
standalone in CI (python test/umep/test_pyqgis_error_handling.py),
so pytest import must be optional.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The `int | None` type hint syntax requires Python 3.10+.
Adding `from __future__ import annotations` allows using this
syntax in Python 3.9.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add SUEWS vendoring documentation to f2py_f90wrap.py explaining
  the SIGINT handler removal for Qt/QGIS compatibility
- Extract QGIS_LTR_PYTHON_VERSION constant to test/umep/__init__.py
  for easier updates when QGIS LTR version changes
- Update test skip reasons to use dynamic version string

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The sys module is still needed for tests that manipulate stdout/stderr.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
sunt05 and others added 9 commits January 13, 2026 15:30
The Fortran supy_error_flag persists between calls and must be reset
before each kernel invocation. Without this reset, an error from a
previous call would cause subsequent calls to exit immediately on the
first timestep check.

This was causing test failures where test_pyqgis_error_handling (which
intentionally triggers an error) left the error flag set, causing
subsequent tests using sample_config.yml to fail unexpectedly.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add supy_version input to allow specifying exact version for qgis_test_only
- Skip QGIS test gracefully when no UMEP variant available on TestPyPI
- Replace emoji with ASCII in workflow output (FAIL/OK/WARN)
- Optimise _check_supy_error_from_state to walk backwards from last timestep

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add test/umep/test_stop_interception.py to verify gfortran STOP
  handlers are present in the compiled extension
- Add symbol inspection step to build-publish_to_pypi.yml and
  test-qgis.yml workflows
- Checks for: suews_stop_string, suews_stop_numeric,
  _gfortran_stop_string, _gfortran_stop_numeric, f90wrap_abort_

Part of GH-1035 validation.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When zzd < 0, ErrorHint sets the error state but execution continued
to the next line which computes LOG(zzd/z0m). LOG of negative values
causes a floating-point exception that crashes the process before
Python can catch the SUEWSKernelError.

This fix adds RETURN after the ErrorHint call to exit cal_Stab
subroutine immediately, allowing the error state to propagate back
to Python and be raised as SUEWSKernelError.

Fixes #1035 (QGIS crash when SUEWS error occurs)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ErrorHint now calls f90wrap_abort for fatal errors, which triggers
longjmp back to the Python/C wrapper. This ensures:

1. Fatal errors jump directly to Python (skip entire Fortran call stack)
2. Python catches RuntimeError instead of process crash
3. No need for callers to add RETURN after ErrorHint

This properly reintroduces the nonstopf2py approach from 2018, where
Fortran errors are converted to Python exceptions via setjmp/longjmp.

Also keeps the defensive RETURN after ErrorHint(32) in cal_Stab as
an additional safety measure.

Fixes #1035

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@sunt05 sunt05 force-pushed the sunt05/gh1035-check branch from 7c00b9f to 6df4e9c Compare January 13, 2026 15:30
sunt05 and others added 9 commits January 13, 2026 16:15
The vendored f90wrap containing STOP interception code was not being
used during build. Three issues prevented it:

1. Missing __init__.py in vendored f90wrap package directory
2. meson.build used find_program() which found PyPI f90wrap
3. run_f2py.py couldn't execute .py scripts directly

Changes:
- Add __init__.py to src/supy/_vendor/f90wrap_src/f90wrap/
- Update meson.build to use f2py-f90wrap-patched.py directly
- Update run_f2py.py to handle .py scripts with sys.executable

This ensures STOP handler symbols (_suews_stop_string, _gfortran_stop_*,
for_stop_core) are included in the compiled extension, enabling proper
error propagation from Fortran to Python without crashes.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Previously the test only warned when critical STOP intercept symbols
were missing, allowing CI to pass even if vendored f90wrap wasn't used.

Now the test:
- FAILS if tools available AND critical symbols missing
- SKIPS only if inspection tools (dumpbin/nm) unavailable
- Returns exit code 2 for build errors in standalone mode

This ensures CI catches build configuration errors where the vendored
f90wrap with STOP handlers is not properly integrated.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move test/umep/test_stop_interception.py to test/core/test_build_symbols.py
- Make test cross-platform (nm works on macOS/Linux/Windows)
- Remove Windows-only skip - now validates on ALL platforms
- Mark as @pytest.mark.core for inclusion in cibw test step
- Remove redundant RETURN in suews_phys_atmmoiststab.f95 (f90wrap_abort handles it)
- Update CI workflow references to new test location

The symbol validation now runs automatically via CIBW_TEST_COMMAND
after each wheel build on all platforms, catching build issues early.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…-1035)

The longjmp mechanism in f90wrap_abort may fail in Qt/QGIS context due to
Qt's signal handling. Without RETURN, execution continues to LOG(negative)
which causes a floating-point exception crash before Python catches the error.

RETURN serves as a safety net - normally longjmp prevents reaching it,
but when longjmp fails (Qt event loop interference), RETURN prevents
the LOG(negative) crash.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The CI workflow runs this script directly in QGIS environment where
pytest may not be installed. Make pytest import optional so main()
can run standalone for symbol validation.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…-1035)

When f90wrap_abort triggers longjmp from Fortran, the C wrapper raises
RuntimeError directly. However, Fortran sets the error state before
calling f90wrap_abort. This fix checks that error state in the except
block and converts the exception to SUEWSKernelError when present.

This ensures the test_pyqgis_error_handling.py test can catch the error
as SUEWSKernelError instead of RuntimeError.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The test triggered error code 14 from SUEWS_cal_RoughnessParameters
instead of error code 32 from stability calculation. Both errors
indicate the same invalid z < zdm configuration, so the test now
accepts either error message.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use error code and simpler string patterns to avoid character
encoding issues when comparing error messages on Windows. The
'<' character may be encoded differently in Fortran strings
passed through f90wrap on different platforms.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Root cause: Fortran I/O operations (WRITE statements, including internal
writes) crash QGIS GUI due to incompatibility with Qt's event loop.

Solution:
- In cal_Stab: bypass ErrorHint entirely and call set_supy_error directly
- In ErrorHint: wrap internal WRITE statements with #ifdef wrf guards
- Remove all WRITE(*,*) statements from non-WRF error handling path
- Use string concatenation instead of internal WRITE for StopMessage

The error is now properly caught and returned to Python via the
supy_error_flag mechanism without crashing QGIS.

Tested successfully in QGIS 3.44.5 on Windows.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

QGIS crash when SUEWS error occurs

2 participants