Skip to content

fix: correct guard order in OsemosysClass.RYE() to prevent phantom Em…#456

Open
lil-aditya wants to merge 1 commit intoEAPD-DRB:mainfrom
lil-aditya:fix/rye-phantom-emisid-key
Open

fix: correct guard order in OsemosysClass.RYE() to prevent phantom Em…#456
lil-aditya wants to merge 1 commit intoEAPD-DRB:mainfrom
lil-aditya:fix/rye-phantom-emisid-key

Conversation

@lil-aditya
Copy link
Copy Markdown
Contributor

Existing related work reviewed

Overlap assessment

Why this PR should proceed

  • The RYE() parser is the only remaining method in OsemosysClass with the inverted guard order. All sibling methods (RYC, RYT, RYTs, RYTTs, RYCTs, etc.) already use the correct pattern. This is a data-corruption bug that creates a phantom 'EmisId' key in the parsed emission data, which causes KeyError crashes in gen_RYE() during solver data file generation.

Summary

  • What changed: Swapped the two guard conditions in OsemosysClass.RYE() (line 608) so the year != 'EmisId' exclusion check runs before the dict-entry creation, matching the canonical pattern used by every other parser method in the class.
  • Why: The original order (if year not in RYE before if year != 'EmisId') creates a phantom empty dict entry RYE[param][sc]['EmisId'] = {} which (1) pollutes the data structure, (2) causes KeyError crashes when gen_RYE() iterates year keys, and (3) can produce corrupted solver .dat files.

Validation

  • Tests added/updated (or not applicable)
  • Validation steps documented
  • Evidence attached (logs/screenshots/output as relevant)

Reproduction steps:

  1. Run python scripts/reproduce_rye_bug.py from the MUIOGO/ root
  2. Observe the [BUGGY] section showing phantom 'EmisId' key and KeyError crashes
  3. Observe the [FIXED] section showing clean year-only keys

Before (buggy):

# Guard order WRONG — creates dict entry before exclusion check
if year not in RYE[param][sc]:
    RYE[param][sc][year] = {}
if (year != 'EmisId'):
    RYE[param][sc][year][o['EmisId']] = val

After (fixed):

Guard order CORRECT — matches RYC, RYT, etc.

if (year != 'EmisId'):
if year not in RYE[param][sc]:
RYE[param][sc][year] = {}
RYE[param][sc][year][o['EmisId']] = val

Copilot AI review requested due to automatic review settings April 21, 2026 09:19
@github-actions github-actions Bot added the needs-intake-fix PR intake structure needs maintainer follow-up label Apr 21, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a data-corruption bug in the OSeMOSYS JSON parser for RYE() by aligning its guard-condition ordering with the established pattern used by sibling pivot/parsing methods in OsemosysClass.

Changes:

  • Reordered the RYE() loop guards so the 'EmisId' exclusion check runs before initializing RYE[param][sc][year].
  • Prevents creating a phantom RYE[param][sc]['EmisId'] = {} entry during parsing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 616 to 619
if (year != 'EmisId'):
if year not in RYE[param][sc]:
RYE[param][sc][year] = {}
RYE[param][sc][year][o['EmisId']] = val
Copy link

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR description references running python scripts/reproduce_rye_bug.py, but that file does not exist in the repository/PR branch. Either add the script to scripts/ or update the validation instructions to point at an existing reproduction path so reviewers can verify the fix.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-intake-fix PR intake structure needs maintainer follow-up

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Phantom 'EmisId' key in OsemosysClass.RYE() due to inverted guard order

2 participants