Keep the years a PLEXOS plan states, and carry its constraints - #10
Closed
thomaschristopherking wants to merge 19 commits into
Conversation
Writes the agreed design before the work that implements it: what makes a PLEXOS object a candidate, which half of the cost PyPSA works out for itself, why one candidate becomes one technology, and why the portfolio is written beside the base system it expands. Three things in the SiennaSchemas investments namespace differ from what the work was scoped against, and each one decides part of the design, so the spec states them: there is no Node or Zone, the existing-fleet records are supplemental attributes rather than components, and TechnologyFinancialData has no field for a weighted average cost of capital. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
The PLEXOS reader named Units and Units Out and nothing else about building, and every capacity mapping wrote p_nom_extendable False on purpose. So a model stating what it may build lost all of it on the first leg. PlexosProperty now names Build Cost, Max Units Built, WACC, Economic Life, Technical Life and FO&M Charge, and Build Cost and FO&M Charge convert from $/kW as they stage. Max Units Built above zero is what makes an object a candidate, and the rule is the same for a Generator, a Battery and a pumped-storage turbine, so one derivation serves all three. PyPSA takes overnight_cost in preference to capital_cost and works the annuity out from the discount rate and the lifetime, so this writes those four fields and assembles no cost of its own. Economic Life is the recovery period PyPSA annuitises across; Technical Life and the size of one unit have no PyPSA field and travel in the extensions sidecar instead. A candidate that has nothing yet takes the capacity it may build as its p_nom. PyPSA reads p_nom only for a component whose capacity is fixed, and it is what every per-unit field on the component is read against, so against nothing each of those would come out at zero. A candidate that states no Build Cost is left out: nothing prices building it, so an expansion would take it for free. Each one is recorded, and one warning names a few of them. Every new column is null for an object the model cannot build, and the sink omits a null column, so the dispatch pipelines write what they wrote before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
The generator path and the storage path each wrote the expansion mapping, and the storage copy inferred the size of one unit from the rated power. That inference was wrong for two of the three classes it served. A pumped-storage turbine's rated power is Max Capacity times Units, so a turbine with two 100 MW units and one more to build got a ceiling of 600 MW where the model offers 300; a battery's rated power is its Max Power alone, so the same expression read one value as two different things and let a solver remove capacity the model states as built. The turbine also never reached the rule at all when it stated Units = 0, because its rated power worked out to zero and the object was dropped as unable to dispatch, which is exactly the candidate the documentation says the translator carries. The rated power now arrives as a pair: what the object already has, and what one unit of it is. Each class states both for itself, so a Generator and a turbine read Max Capacity as one unit and multiply by Units, and a Battery reads Max Power. One module, _expansion.py, holds the rule the three share: the extendable flag, the floor, the ceiling, the build cost, the discount rate, the lifetime, the fixed cost, and the two values that travel in the sidecar. The floor is the rated power the object has, and the ceiling adds Max Units Built units of one unit's rated power to it. A candidate with nothing built takes the capacity it may build as its p_nom, on the storage path as it already did on the generator path. A mapping dataclass holds one ExpansionDecisions field and adopts its column declarations, so the reported events and the destination row still come from one walk. A candidate that states a Build Cost but no WACC is now left out with the Build Cost case. PyPSA's cost consistency check raises whenever an overnight_cost is set and the discount rate is missing, whatever the strict setting, and Network.optimize runs that check, so such a network cannot be solved at all. On the PyPSA to Sienna leg, base_power and the active power limits took p_nom_opt from any extendable component. An unsolved network states zero there, so every PLEXOS candidate this branch now writes would have translated at zero capacity. The generator, renewable, storage and hydro mappings now guard that choice with p_nom_opt above zero, as the link mapping already did and as the mapping document already described. Two generator scenarios asserted derivation text that named a generator; the shared rule names an object, and they now assert that. Two scenarios stating a Build Cost and no WACC gained one, since the translator no longer writes a network PyPSA refuses. New scenarios cover a pumped-storage turbine that runs two units and may build a third, and one that has yet to build anything. Addresses r1-01, r1-02, r1-03, r1-07, r1-08, r1-09, r1-10, r1-11, r1-15, r1-16, r1-17, r1-18, r1-19, r1-20, r1-21, r1-22, r1-23, r1-24, r1-25, r1-26, r1-27, r1-29, r1-30 and r1-31. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
code-review/ is committed with the code it judges, so a teammate and a fresh clone start with the rules this repository has earned rather than with none. The README says what each file in it is for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
…ne owner The effective_p_nom rule stood in seven places across the PyPSA leg, and each copy read a p_nom_opt of zero as an unsolved network. A solve that builds none of an extendable component writes p_nom_opt 0; only a network no solve has touched leaves the column out. Reading that 0 as unsolved carried the whole candidate capacity into the Sienna operations system, adding a plant the expansion refused. The rule now lives once in the pypsa_sienna_translations _shared module, keyed on whether the network states a p_nom_opt at all, and the generator, renewable, hydro, storage and link modules call it. The four fill_defaults calls leave p_nom_opt null rather than filling it to zero, so an unsolved network is still distinguishable. A candidate battery divided its stated Capacity by the capacity it may build, so its max_hours came out Max Units Built times too small. A Battery states its Capacity beside one unit's Max Power, so both now per-unitise against that one power, and the initial state of charge reads its percentage against p_nom times max_hours rather than against one unit's energy. A candidate that states no Economic Life is left out beside one that states no Build Cost or no WACC. PyPSA's lifetime default is infinity, not NaN, so nothing refuses the network and the build is annuitised as a perpetuity; a candidate stating WACC 0 with no Economic Life would be built for free. The rest is duplication and dead code. SourceGenerator no longer restates the candidate rule in plain floats: it reads its p_nom off the Decision _expansion builds, so the reported event and the emitted row cannot disagree. The warning about an unpriced candidate moves beside that rule, so the generator path and the storage path both cap it at a few names instead of the storage path warning once per object. NOT_A_CANDIDATE becomes NOTHING_TO_REPORT, which is what a genuine candidate stating no FO&M Charge also gets. ExpansionDecisions.is_candidate and RatedObject.stated_units had no reader, and three docstrings restated their own signature or a rule the code does not have. The scenarios that reached the expansion path with a Build Cost and a WACC alone now state an Economic Life, and one more candidate stands beside them to show what leaving it out does. Two new assertions pin the numbers the fixes change: a candidate battery's max_hours and initial level, and the base_power of an extendable generator a solve built none of. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
…ispatch system PLEXOS states a Battery's Max Power per unit, as the mapping document's own account of what a candidate is already said, so a Battery running two units now reaches PyPSA at twice that power rather than once. The units-out trace derates p_max_pu by units_out / units, so p_nom has to stand for every unit before that derate means anything. A Battery stating Units 0 and no Max Units Built now works out to no rated power and is left out, with the reason recorded and warned about, as a turbine with no units already was. (r3-01, r3-08) The pypsa-to-sienna leg leaves out an extendable component no solve has sized. PyPSA ignores the p_nom of an extendable component, and the network the PLEXOS leg writes puts the capacity a candidate may build there, so reading it back gave a Sienna operations system a plant nobody has built. A component a solve has sized still translates at its p_nom_opt. (r3-06) One carrier now reports every PLEXOS skip. SkippedComponent moves to decisions.py and carries the group a warning speaks for, so the unpriced-candidate rule needs neither a return type of its own nor a field of its own, and every generator skip warns as a storage skip already did. (r3-02) Deletes SourceGenerator.existing, which nothing read (r3-11), and two docstrings that restated the line below them (r3-09, r3-10). The mapping documents now say which reading of Max Power is correct, that a turbine's p_nom is Max Capacity times Units and nothing else, that a candidate stating Units 0 is written rather than skipped, and that a candidate does not reach a Sienna operations system. (r3-13, r3-14) The scenarios follow the behaviour. The battery p_nom event cites Units beside Max Power. The three scenarios asserting that p_nom_extendable round-trips into extensions.json give their component a p_nom_opt, since a network no solve has touched no longer translates its candidates. The infeasible-dispatch warning is now the note the skip records, which says "sits above" where the warning it replaces said "is above". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
A dated Max Capacity becomes a p_max_pu series, scaled so the band in force reads as a share of p_nom. The scale was the unit count the object runs, which a candidate states as zero, so every snapshot of the band held the candidate at zero and a solve could build none of it. The unit count p_nom stands for is what the object runs, or what a candidate running none of them may build, which is the reading derive_p_nom already takes, so CandidateSource states it once and the dated rows scale by it. A scenario covers a candidate whose capacity rises during the year: it fails against the old scale and passes against the new one. Addresses r4-01 and r4-04. _drop_unbuilt_candidates had one caller and two working lines, so they move into _prepare_source beside the SkipRule it already applies (r4-09). The reason the deleted docstring gave stands in unbuilt_candidate_skip, so no comment replaces it (r4-06). Drops the paragraph defending the grouped warning in find_unpriced_build (r4-07), and the two feature comments repeating the decisions.md text the same scenario asserts (r4-08). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
A Generator, a Battery or a turbine that states a Max Units Built the model prices nothing for was left out whole, so a plant already generating vanished from the network because no expansion of it was priced. An object with units in service now keeps the capacity it runs, with that capacity fixed, and only the build is left out: decisions.md carries a NOT_MAPPED row naming the property the model left out, and one warning names a few of the objects it happened to. An object with no units yet is the build and nothing else, so it is still skipped. (r5-04) The battery's state_of_charge_initial named Capacity and Initial SoC while its value came from the energy of every unit. The energy the battery holds is now a Decision of its own, so p_nom and max_hours travel with the level and the derivation states what the code does. (r5-01, r5-09) Name a case the zero guard in the Sienna storage initial level can still meet, since an extendable unit no solve has sized is now dropped before it. (r5-10) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
RatedPower carries the PLEXOS class and the capacity property, which is everything the two rating functions differed by, so the rating becomes a method on it and the callback field goes. The derivation string still reads "Max Power * Units" and "Max Capacity * Units", built from the property the constant names (r6-01). Drops two docstrings that repeat what the name beside them already says (r6-08, r6-09), and counts the skipped generator cases the mappings table actually lists (r6-10). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
The pypsa-to-sienna scope rule read p_nom_extendable and p_nom_opt and nothing else, so it left out every extendable component no solve had touched. A plant that already generates and merely states what it may build states that running capacity as p_nom_min, and PyPSA ignores the p_nom of an extendable component, so the rule deleted real capacity from the Sienna system. The PLEXOS leg writes exactly that shape for a plant with a Max Units Built. A component belongs in the operations system when it holds capacity an operations model may dispatch: any fixed component, an extendable one a solve has sized, and now an extendable one whose p_nom_min is a capacity a build cannot take away. effective_p_nom rates it at that floor, and the events name p_nom_min as the attribute the rating came from. Only an extendable component with neither a p_nom_opt nor a p_nom_min is a build the plan has yet to decide, and that one is still left out and named in decisions.md. (r7-01) Links read the same rating, so an extendable link with a floor now rates at it rather than at a p_nom PyPSA never reads. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
The ledger is committed with the code it judges, so a rule earned here is a rule a teammate and a fresh clone start with. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
…ndings' into issue-116-plexos-pypsa-sienna-capacity-expansion-portfolio Signed-off-by: Thomas C. King <thomas.k@transitionzero.org> # Conflicts: # tests/features/plexos_to_pypsa/generators.feature
A PLEXOS expansion plan writes a schedule of dated Units: an object runs none of itself until the year it is built, and its units fall back to zero in the year it retires. The staging layer narrowed every dated property to the value in force when the window opened and discarded every band edge outside it, so a generator built in 2032 read as no units at all in a 2026 window and the year itself was gone. The source now stages every resolved row a third time, unclipped and beside the dates it applies between. The generator and battery mappings read the Units bands off that table: the first year the units rise above zero becomes the build_year PyPSA carries on the component, and the first year after that in which they fall back to zero becomes the retirement_year in the extensions sidecar, since PyPSA has no field for the other end of a plant's life. A Constraint was read in full -- its sense, its right-hand sides, the objects it names and the coefficients weighting them -- and then reported as carried nowhere. PyPSA's GlobalConstraint limits one carrier over the whole horizon and cannot name a set of components, which is the ground on which a reserve already travels in the sidecar, so a constraint now travels there too: its sense as the inequality it holds in, each right-hand side beside the span it applies over, each member with the class its name belongs to and the coefficient weighting it, and whether the expansion plan has to meet it. One stating no sense, or no right-hand side at all, states no inequality to carry, so it is still left out and reported. The operations leg relays the records on unchanged, so nothing reports them as dropped on the way to Sienna. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
thomaschristopherking
left a comment
thomaschristopherking
left a comment
Contributor
Author
There was a problem hiding this comment.
Review by /review-fix-cycle.
Labels follow Conventional Comments. (blocking) means the reviewer would not merge without it settled. (non-blocking) means it can land after.
r1-01: `plexos_dates` now holds `latest_covering`, `band_edges` and `opens_at`, and both the source's window narrowing and the lifespan reading call them. The rule about what a band covers, and the rule that an undated band sorts first, each stand in one place. r1-02, r1-14, r1-16: the generator path derives the retirement year once and reads the sidecar value off that decision, as the storage path and the expansion fields already do. `Lifespan.build_year` and `Lifespan.retirement_year` go with the second route. The reviewer's suggestion of moving the derivation into the storage report does not work: that loop carries both batteries and hydro turbines, and the two name different PLEXOS classes. r1-03: `dated_rows` now stages only the rows of a property the model dates somewhere. Staging every band of every property costs a multiple of the `properties` table on a model with monthly bands, and an undated property states the same value for all time, which `properties` already carries. Keeping every dated property rather than only `Units` leaves the table usable by the next reader. r1-05: `ConstraintExtension.sense` takes a default, since `ExtensionLookup.get` builds an empty record for a name the sidecar never mentioned and promises to answer with a record rather than raise. r1-17: the note against a Constraint that is left out now says why it is left out. It stated that PyPSA's GlobalConstraint cannot express the limit, which is no longer the reason anything is dropped, since a readable Constraint reaches the sidecar. r1-08, r1-09, r1-10, r1-11, r1-12, r1-13: delete docstrings that repeat a name, argue for a module, narrate the line below, or say a third time what the constants above already said. r1-15: delete the unread `BUILD_YEAR` from `PyPSALineCol` and `PyPSALinkCol`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
thomaschristopherking
left a comment
thomaschristopherking
left a comment
Contributor
Author
There was a problem hiding this comment.
Review by /review-fix-cycle.
Labels follow Conventional Comments. (blocking) means the reviewer would not merge without it settled. (non-blocking) means it can land after.
The build year and the retirement year are two ends of one concept, so they now travel as one LifespanDecisions bundle beside the ExpansionDecisions bundle: derive_lifespan states both ends, record_lifespan records the end the sidecar carries, and each mapping declares one field. The generator path loses the local dict of retirement years it built inside _carry_to_extensions and reads the ends off the decisions it already derived (r2-01). An undated band covers every moment, so latest_covering always finds one where the property states any, and the fallback to the undated row could never run. It goes, together with the parameter and the helper feeding it (r2-02). A Sense that is not -1, 0 or 1 read as no Sense at all, so the report told the reader the model stated nothing where it stated something the translator cannot read. The Constraint keeps the code the model stated, the note says the code is unreadable, and the description names it. A scenario covers it. The map_constraints docstring said the step carries every Constraint, which a Constraint stating no sense or no right-hand side does not (r2-11), and the two sidecar scenarios in constraints.feature built the same model twice, so the assertions of the second now stand on the first (r2-12). Comments and docstrings that restated the code they sit on are gone: the ConstraintPeriod paragraph, the two build_year schema comments, the _states_a_date and latest_covering docstrings, and the _PERIODS heading (r2-06, r2-07, r2-08, r2-09, r2-10). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
thomaschristopherking
left a comment
thomaschristopherking
left a comment
Contributor
Author
There was a problem hiding this comment.
Review by /review-fix-cycle.
Labels follow Conventional Comments. (blocking) means the reviewer would not merge without it settled. (non-blocking) means it can land after.
… not do StorageLookups stated the same five readings twice, once for a Battery and once for a Generator. One ClassLookups per PLEXOS class holds them instead, and staged(plexos_class, name) builds the StagedObject from whichever class the caller names, so a new per-object reading costs one field rather than two. The Units row of the PLEXOS to PyPSA mapping claimed a component running no units in the year translated is left out. It is not: derive_p_nom gives a candidate its buildable capacity where its existing rating is zero, so the component is written with the build_year the schedule states. The skipped-component table already gives the real rule, so the sentence goes. _states_a_date guarded nothing. A band list that is all undated yields no band edge, so it already read as NO_LIFESPAN, which is what both callers fall back to for a name the map does not hold. Three docstrings that only paraphrased what they sat above go with them. Addresses r3-01, r3-03, r3-04, r3-05, r3-06, r3-07 and r3-08. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Thomas C. King <thomas.k@transitionzero.org>
thomaschristopherking
added this pull request to stack #13
September 9, 2026 09:58
thomaschristopherking
added
review-fix-running
thomaschristopherking
added
review-fix-running
thomaschristopherking
added
the
review-fix-running
…pacity-expansion-portfolio' into issue-116-plexos-dated-units-and-constraints # Conflicts: # code-review/ledger.jsonl # docs/specs/2026-09-08-plexos-sienna-investments-design.md # docs/translation_mappings/translation-from-plexos-to-pypsa.md # docs/translation_mappings/translation-from-pypsa-to-sienna.md # interop/plugins/shared/plexos_pypsa_translations/_batteries.py # interop/plugins/shared/plexos_pypsa_translations/_expansion.py # interop/plugins/shared/plexos_pypsa_translations/_generator_decisions.py # interop/plugins/shared/plexos_pypsa_translations/_generator_derivation.py # interop/plugins/shared/plexos_pypsa_translations/_generators.py # interop/plugins/shared/plexos_pypsa_translations/_storage_hydro.py # interop/plugins/shared/plexos_pypsa_translations/_storage_shared.py # interop/plugins/shared/plexos_pypsa_translations/_storage_units.py # interop/plugins/shared/plexos_pypsa_translations/decisions.py # interop/plugins/shared/pypsa_constants.py # interop/plugins/shared/pypsa_sienna_translations/__init__.py # interop/plugins/shared/pypsa_sienna_translations/_generators.py # interop/plugins/shared/pypsa_sienna_translations/_hydro.py # interop/plugins/shared/pypsa_sienna_translations/_links.py # interop/plugins/shared/pypsa_sienna_translations/_renewables.py # interop/plugins/shared/pypsa_sienna_translations/_shared.py # interop/plugins/shared/pypsa_sienna_translations/_storage.py # interop/plugins/sinks/emit_pypsa_network.py # tests/features/plexos_to_pypsa/generators.feature # tests/features/plexos_to_pypsa/storage_units.feature # tests/features/pypsa_to_sienna/thermal_generators.feature
|
Gate: ✅ PASS All files grade A. Full report
|
Mutation testing reportScore: 79.4% (259 killed / 326 tested)
Timeout-prone functionsEach timeout burns its whole budget (
|
Round 1 of the automatic review cycle. Addresses r1-01, r1-02, r1-03, r1-04, r1-05, r1-06, r1-08 and r1-10. r1-01: the merge with the parent branch left the unbuilt-candidate rule in two places. _rows_in_scope already applies it to the same rows, so the copy in _prepare_source dropped nothing and cost one Polars pass per mapping. r1-02: a schedule whose Units return above zero after a dated zero band read as a build in the return year. A generator running two units with a dated zero from 2035 to 2040 reported build_year 2041, although it runs in the year translated. An object that runs before its first dated change now states no build year, and one that runs again after a zero band states no retirement year. A new scenario covers it, and the retirement label says "the last year" because the rule now takes the last fall to zero. r1-03: the Units row of the mapping document named a shape the translator drops. A generator that runs no units and states no Max Units Built is left out as retired, so no build_year reaches the network for it. r1-04: GeneratorExtension and StorageExtension declared unit_size_mw and technical_life_years again, which they inherit from ExpansionExtension. retirement_year moves to the base class, where both records need it. r1-05: _expansion.py holds the sidecar keys as constants now, rather than as literals beside the constants that spell the same three strings. r1-06: StorageLookups answers properties_of(plexos_class), so a caller stops reaching through by_class. The docstring says that by_class holds the Battery and the Generator alone. r1-08 and r1-10: two docstrings that paraphrase the name they sit on go. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
thomaschristopherking
left a comment
thomaschristopherking
left a comment
Contributor
Author
There was a problem hiding this comment.
Automatic review-and-fix, round 1. Four reviewers ran over the diff against issue-116-plexos-pypsa-sienna-capacity-expansion-portfolio. Ten comments after merging the duplicates. I acted on eight and deferred two. Commit f982f34 carries the fixes.
| id | file | claim | severity |
|---|---|---|---|
| r1-01 | interop/plugins/steps/pypsa_to_sienna_map_components.py:311 |
_prepare_source applies unbuilt_candidate_skip a second time to rows _rows_in_scope already filtered with the same rule. |
critical |
| r1-02 | interop/plugins/shared/plexos_pypsa_translations/_lifespan.py:147 |
A schedule whose Units return above zero after a dated zero band is read as a build in the return year. |
major |
| r1-03 | docs/translation_mappings/translation-from-plexos-to-pypsa.md:782 |
The Units row names a shape the generator mapping drops before any build_year is written. |
major |
| r1-04 | interop/core/extensions.py:149 |
GeneratorExtension and StorageExtension redeclare two fields they inherit from ExpansionExtension. |
major |
| r1-05 | interop/plugins/shared/plexos_pypsa_translations/constants.py:21 |
Two sidecar field constants have no reader, and _expansion.py holds the same strings as literals. |
major |
| r1-06 | interop/plugins/shared/plexos_pypsa_translations/_storage_shared.py:200 |
A caller reaches through by_class for a battery's properties, and by_class holds no Storage entry. |
major |
| r1-08 | interop/plugins/shared/plexos_pypsa_translations/_constraints.py:264 |
The docstring paraphrases the function name. | minor |
| r1-10 | interop/plugins/shared/plexos_pypsa_translations/_lifespan.py:60 |
The docstring restates the field types. | nit |
Two comments are deferred, not rejected. A rejection states the reader's taste, and this run has no reader in it.
- r1-07
_constraints.py:106(minor, thermo):_Constraintkeeps the parsed sense beside the raw code, and three functions read the pair to decide which of three states the constraint is in. A_Sensevalue holding the value, the text andis_unreadablewould state the three cases once. - r1-09
_lifespan.py:3(nit, pruning): the first sentence of the module docstring packs both rules into one clause chain.
r1-01 and r1-04 are artefacts of the merge with the parent branch: this branch carried older copies of the parent's own commits, so the merge kept a rule and two fields in two places each.
thomaschristopherking
removed
the
review-fix-running
Contributor
Author
|
Superseded by #14, which squashes this whole stack into one branch off |
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
1 participant
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.
TL;DR
A PLEXOS plan states when each unit arrives and leaves in a dated
Unitsproperty, and whatthe plan must meet in its
Constraintobjects — 604 dated generator rows and 186 Constraintsin the AEMO 2024 ISP file alone.
stage_plexos_xmlnarrowed every dated property to the oneyear being translated and
map_constraintsreported every Constraint as not carried, so allof it stopped at the first leg. The source now stages every date band beside the value in
force, and a readable Constraint reaches the extensions sidecar.
Second of four for transition-zero/tz-infra-interop#116,
stacked on #9.
Files to review (24, +692 / -73):
interop/plugins/shared/plexos_pypsa_translations/_lifespan.py(new, start here)Unitsbands for the build year and the retirement year.interop/plugins/sources/plexos_dated_properties.pyinterop/core/extensions.pyExtensionKind.CONSTRAINTand the four models behind it.interop/plugins/shared/plexos_pypsa_translations/_constraints.pyinterop/plugins/steps/pypsa_to_sienna_map_components.pyinterop/plugins/shared/plexos_dates.py(new)DateBand, which the source and the mapping both read.Why
apply_windowreturns the value each property held when the window opened, and_change_momentskeeps only the band edges inside that window. A generator built in 2032therefore reads
Units = 0in a 2026 run and is dropped as retired, and a generator thatretires in 2035 says nothing about it. Neither year survives to tell an expansion when
capacity arrives or leaves.
A
Constraintfared worse:map_constraintsread the sense, the right-hand sides, themembers and their coefficients, and then reported every one as not carried. PyPSA's
GlobalConstraintlimits one carrier over the whole horizon and cannot name a set ofcomponents, so an emissions budget over 40 named plants has no PyPSA field at all.
How
The source keeps a third table.
apply_windowstill returns the value in force and the stepswithin the window; alongside them the source now stages every resolved band with its
date_fromanddate_to, underPlexosResolvedTable.DATED_PROPERTIES. It is one row perobject per band, so it is component-scale and safe to collect.
build_yearrides the hub, because PyPSA carries it on Generator, StorageUnit, Line andLink and it survives the netCDF round trip as a real field. PyPSA has no retirement year, so
that travels in the sidecar — the same grounds that already put a reserve there.
A Constraint reaches the sidecar as a record rather than a PyPSA component, and the
operations leg relays it unchanged.
ExtensionKind.RESERVEis the precedent: the conceptexists in PLEXOS and in Sienna and not in PyPSA, so the sidecar is what gets it across the
hub.
Reviewer notes
DateBandmoved tointerop/plugins/shared/plexos_dates.py. The import contractsforbid a step reaching into
interop.plugins.sources, and the mapping now needs the samereading of what a band covers that the source has.
weight a Generator and an Emission in the same sum, so a name alone does not identify what
is being weighted.
Generators, so they read the same dated
Units. The mapping document states the ruleagainst the component the translation writes rather than repeating the row in three tables.
build year reaches the network only when it is also a candidate. Changing that skip belongs
with the investments leg, not here.
One log assertion and one
decisions.mdassertion moved with it.Tests
Five new scenarios across
dated_properties.featureandconstraints.feature: a build year,a retirement year, an object with neither, a Constraint reaching the sidecar with its sense,
its limits and its weighted members, and a Constraint left out for stating no sense. Full
suite (605), ruff, mypy, the import contracts and every plugin lint are green.
CI does not run on this pull request: the workflows trigger on a pull request into
main,and this one targets the branch below it in the stack.
Follow-up
The
pypsa-to-sienna-investmentsleg and the portfolio sink, which read these records.Open workspace in Conductor