Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix variable scenario filtering, and availability TS multiplier #91

Merged
merged 8 commits into from
Feb 8, 2025

Conversation

ktehranchi
Copy link
Collaborator

Fixes:

  • Fixes scenario filtering of variables by Band ID. We could potentially apply this filter to other Enum classes, but I didn't want to unnecessarily filter out data unless we knew that behavior was needed:

    R2X/src/r2x/parser/plexos.py

    Lines 1249 to 1258 in ebac92c

    # Filtering Variables to select only the correct band-id which should be used
    # when accessing Variable data. We may also need to apply this filtering to
    # other Enum classes.
    variables = data.filter(pl.col("child_class_name") == ClassEnum.Variable.name)
    variables = variables.group_by("object_id", maintain_order=True).map_groups(
    lambda groupdf: groupdf.filter(pl.col("band") == pl.col("band").min())
    )
    data = data.filter(pl.col("child_class_name") != ClassEnum.Variable.name)
    data = pl.concat([data, variables])

  • One of the paths for parsing TS data was missing a * record['available']

@ktehranchi ktehranchi requested a review from pesap November 22, 2024 19:03
@pesap pesap requested a review from JensZack November 22, 2024 19:19
@codecov-commenter
Copy link

codecov-commenter commented Nov 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.12%. Comparing base (f926bb4) to head (376f77d).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #91      +/-   ##
==========================================
+ Coverage   78.49%   81.12%   +2.63%     
==========================================
  Files          40       40              
  Lines        4078     4143      +65     
==========================================
+ Hits         3201     3361     +160     
+ Misses        877      782      -95     
Files with missing lines Coverage Δ
src/r2x/api.py 82.22% <100.00%> (ø)
src/r2x/exporter/sienna.py 94.97% <100.00%> (+0.05%) ⬆️
src/r2x/parser/plexos.py 64.00% <100.00%> (+10.27%) ⬆️

... and 5 files with indirect coverage changes

@JensZack
Copy link
Collaborator

This looks good to me, but we should probably add a test to cover this case. Do we have a dataset we can run that hits these changes?

@ktehranchi
Copy link
Collaborator Author

So we don't have a database that covers this test case. I don't have access to plexos to make one. One idea would be to use a more complicated public file like the caiso's deterministic plexos file as a test case. But r2x fails to convert this atm.

Copy link
Collaborator

@pesap pesap left a comment

Choose a reason for hiding this comment

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

LGTM! Do you want to go ahead and merge the other branch before merging to main?

goroderickgo and others added 2 commits February 8, 2025 00:52
… output files (#122)

1. Added simple filtering logic to `PlexosParser._construct_buses()` to
exclude islanded buses from being parsed from a `plexos` xml file,
because islanded buses don't work in `Sienna`
2. Added simple sorting logic to output CSVs so that files created by
`r2x` are can be more easily diff'd (previously, it seemed that `r2x`
parsing was not deterministic, so running `r2x` on the same xml file
would result in different orderings in output files.
3. Updated the `tables` dependency based on my own experience trying to
set up `r2x` in January (can roll this back/ignore if addressed
elsewhere)

---------

Co-authored-by: pesap <[email protected]>
@pesap pesap linked an issue Feb 8, 2025 that may be closed by this pull request
@pesap pesap merged commit 0b37690 into main Feb 8, 2025
8 checks passed
@pesap pesap deleted the kt/fix_scenario_filtering branch February 8, 2025 08:10
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.

chore: Create a more comprehensive Plexos dummy data for testing
5 participants