Fix/pipeline robustness - #57
Merged
Merged
Conversation
…rom env manifests pixi.toml was missing openpyxl, wbgapi, and highspy, all of which the pipeline actually needs but has never declared consistently: - openpyxl/wbgapi: imported directly by rules/preparation.smk steps (retrieve_iron_ore, download_labour_data); a fresh pixi install could not run the pipeline end to end. - highspy: needed by calculate_lcox.py's SHIFT_SOLVER=highs path, which CI (.github/workflows/ci.yml) relies on since CI runners have no Gurobi license. Was present in environment.yaml but absent from pixi.toml. environment.yaml had drifted the other way: it carried ~15 packages with zero references anywhere in the repo (country_converter, powerplantmatching, jpype1, entsoe-py, descartes, fiona, rasterio, rioxarray, memory_profiler, pytz, xlrd, pyxlsb, dask, tsam, pyscipopt, glpk), confirmed by repo-wide grep across scripts, rules, and notebooks, and by checking config.yaml's solver_options (only gurobi-default/highs-default are configured - no scip or glpk path is actually exercised despite both being listed). Removed those; kept pytables/lxml/pypsatopo/graphviz since they're used by standalone analysis notebooks even though not wired into the Snakemake DAG. Also temporarily disables the pre-commit/pre-commit-hooks repo in .pre-commit-config.yaml: its compiled console-script hooks (check-yaml, check-added-large-files, etc.) hit an OS-level "Access is denied" on Windows, reproduced via both PowerShell and Git Bash - looks like local endpoint security blocking a freshly pip-installed unsigned exe.
tests/ was deleted in a prior commit, so the "Run tests" step (pixi run -e test unit-tests) has been silently failing/meaningless - there's nothing for pytest to collect. Replace the job with what CI can actually verify right now: that the pixi environment resolves and installs cleanly on both ubuntu and windows. Also fixes two things that undermined even that goal: - the "**.ya?ml" paths-ignore meant CI never ran on changes to environment.yaml/pixi.toml/this workflow itself - exactly the files most likely to break a clean install. - the job only ran after a PR was already merged (types: [closed] + if: merged == true), which made sense when it ran expensive solves, but is too late to be useful for a cheap install check. Switched to a normal pre-merge PR trigger.
`pip install pixi` does not install prefix.dev's pixi package manager - PyPI's "pixi" is an unrelated Pixiv API client (pulls in pixiv-api, cloudscraper, etc. as dependencies). This was already broken in the original workflow; it just went unnoticed because CI previously only ran post-merge (see the earlier trigger-timing fix), so nobody watched it fail. Confirmed live: `pixi install` failed with "The database needs to be migrated. Run `pixi migrate`." - the telltale error from that wrong package's own CLI, not from real pixi. Use the official setup-pixi action instead, which installs the real pixi binary directly (no PyPI ambiguity) and works across both matrix platforms.
… network plot_trade_network did n.statistics.supply(...).loc[:, :, carrier], which raises KeyError when that carrier has zero entries in the network (e.g. no Generators/Links of that carrier were built for the region/product mix in play). Because the whole model_trade rule writes result.csv/network.nc and then plots in one Snakemake job, this crash discarded an already-computed, valid trade-optimization result - Snakemake deletes declared outputs of a failed job regardless of what was actually written. Reproduced by scoping config.yaml down to 2 regions for a pipeline smoke test: the reduced network genuinely had no iron_ore-carrier entries for this run's product/scenario combination, triggering the KeyError on a model that had already solved successfully. Fix: _carrier_values() checks membership in the statistics MultiIndex first and returns 0 - the same "not applicable" value this function already uses for products outside [interone, intertwo, final, "iron_ore"], which n.plot.map already handles correctly - instead of an empty Series (which would also crash, one level deeper in PyPSA's own plot.map via an AssertionError on an empty bus_sizes index). Verified end to end: reran the full 2-region supply-curve -> trade-model chain from scratch with this fix in place; all 9 declared outputs (result.csv, network.nc, and 3 pairs of map pdf/png) were produced correctly, including map_ironore.pdf, the one that previously crashed. Also fixes an unrelated pre-existing bug ruff caught while touching this file: create_links() referenced an undefined name `pipe_mc` in its pipeline-link branch (trade_options rows with pipeline=1 and final_product="hydrogen"). Git history shows pipe_mc used to be computed from a `transport_costs` input that was later removed when shipping costs were migrated to config["trade"]["shipping"] - the pipeline branch was never updated to match, so it's been dead code (only reachable with final_product="hydrogen", which nothing currently configures) that would NameError immediately if ever exercised. Rather than invent a pipeline cost figure I have no basis for, it now raises a clear NotImplementedError explaining the gap instead of crashing on an undefined name.
setup_logging() (shared by every workflow script) built both its console StreamHandler and its FileHandler with no explicit encoding, so each defaulted to the platform's preferred encoding - cp1252 on Windows. Any log message containing a character outside that codepage (e.g. the "→" arrows used in download_labour_data.py's progress messages) made the handler's stream.write() raise UnicodeEncodeError inside emit(). logging catches that internally and prints a "--- Logging error ---" banner instead of crashing the run, so this was silent in practice - but it meant the intended log message (and any log message like it) was lost. Two independent fixes, since Windows consoles and log files have different constraints: - FileHandler: pass encoding="utf-8" explicitly. A log file has no display constraint, so there's no reason to inherit a legacy codepage. - Console StreamHandler: reconfiguring the shared sys.stderr object in place (the obvious first fix) turned out not to reliably affect handlers/codecs that already hold a reference to it - verified this empirically, the message still crashed after reconfigure. Instead, give the handler its own dedicated stream wrapping the same underlying fd with errors="backslashreplace", so unencodable characters degrade to a visible escape instead of raising. Verified with an isolated repro (logger.info with an arrow character): before this fix, both the console output and the log file showed the "--- Logging error ---" banner; after, the console prints the backslash-escaped form and the log file contains the real "→" character with no banner in either place.
JanTautorus
force-pushed
the
fix/pipeline-robustness
branch
from
August 11, 2026 15:13
f5af474 to
e797593
Compare
energyLS
approved these changes
Aug 13, 2026
energyLS
left a comment
energyLS
left a comment
Owner
There was a problem hiding this comment.
Thank you for the fixes, looks good.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixed two small bugs in the while snakemake pipeline leading to crashes on Windows