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.

Sign up for GitHub

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

Jump to bottom

fix electricity emissions column being used by total with mid case sum #246

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

fix electricity emissions column being used by total with mid case sum #246

wants to merge 13 commits into from

Conversation

mpraprost
Copy link
Contributor

Dicsovered this issue a few months ago and realized I never committed it. The total_with_mid_case emissions result was actually using the LRMER 95 decarb by 2035 electricity emissions, so the totals were off.

Pull request overview

  • Fixes #ISSUENUMBERHERE (IF RELEVANT)

Pull Request Author: Marley

This pull request makes changes to (select all the apply):

  • Documentation
  • Infrastructure (includes apptainer image, buildstock batch, dependencies, continuous integration tests)
  • Sampling
  • Workflow Measures
  • Upgrade Measures
  • Reporting Measures
  • Postprocessing

Author pull request checklist:

  • Tagged the pull request with the appropriate label (documentation, infrastructure, sampling, workflow measure, upgrade measure, reporting measure, postprocessing) to help categorize changes in the release notes.
  • Added tests for new measures
  • Updated measure .xml(s)
  • Register values added to comstock_column_definitions.csv
  • Both options_lookup.tsv files updated
  • 10k+ test run
  • Change documentation written
  • Measure documentation written
  • ComStock documentation updated
  • Changes reflected in example .yml files
  • Changes reflected in README.md files
  • Added 'See ComStock License' language to first two lines of each code file
  • Implements corresponding measure tests and indexing path in test/measure_tests.txt or/and test/resource_measure_tests.txt
  • All new and existing tests pass the CI

Review Checklist

This will not be exhaustively relevant to every PR.

  • Perform a code review on GitHub
  • All related changes have been implemented: data and method additions, changes, tests
  • If fixing a defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
  • Reviewed change documentation
  • Ensured code files contain License reference
  • Results differences are reasonable
  • Make sure the newly added measures has been added with tests and indexed properly
  • CI status: all tests pass

ComStock Licensing Language - Add to Beginning of Each Code File

# ComStock™, Copyright (c) 2023 Alliance for Sustainable Energy, LLC. All rights reserved.
# See top level LICENSE.txt file for license terms.

wenyikuang and others added 13 commits September 24, 2024 16:20
@wenyikuang
sight glass check
0f73f8c
@wenyikuang
Check the colnms null and etrc.
7d47e3d
@wenyikuang
Refactored partial of the logic of meta check and skip sererval columns.
191d216
@mdahlhausen
add rubocop checks
a1bb0b8 
rubocop was already included in the gemspec. This adds a rake task to run rubocop on all ComStock ruby files.
@JanghyunJK
temp
fb8d638
@mdahlhausen
Update rake file and measure test rake tasks
3087057 
- split measure tests into reporting, workflow, and upgrade
- enforce rubocop standards on Gemfile and Rake file
- update README.md
@mdahlhausen
Merge branch 'develop' into mdahlhau/rubocop
b110584
@LandanT
Merge pull request #227 from NREL/mdahlhau/rubocop
92f46d3 
Add rubocop rake task
@rHorsey
Merge pull request #224 from NREL/wenyi/comstock-check-sg-meta
1f5866c 
[WIP] Check the metadata in the postprocess when the pl.data in the memory
@mdahlhausen
fixes to documentation
86ed0ef 
latex fixes to figures, references
adding soil properties section
updating kitchen equipment power values
@mdahlhausen
Merge remote-tracking branch 'origin/rhorsey/patch_release_2024_1_aut…
f8434b6 
…horship' into documentation-fixes
@rHorsey @mdahlhausen
Merge pull request #161 from NREL/documentation-fixes
321f77a 
fixes to documentation
fix electricity emissions column being used by total with mid case sum
fbe218a 
total_with_mid_case result was actually using the LRMER 95 decarb by 2035 electricity emissions, so the totals were off
eringold
eringold previously approved these changes Oct 29, 2024
View reviewed changes
@mdahlhausen mdahlhausen changed the base branch from develop to main October 30, 2024 16:09
@mdahlhausen mdahlhausen dismissed eringold’s stale review October 30, 2024 16:09

The base branch was changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eringold eringold eringold left review comments

@ChristopherCaradonna ChristopherCaradonna Awaiting requested review from ChristopherCaradonna

At least 1 approving review is required to merge this pull request.

Assignees

@eringold eringold

@ChristopherCaradonna ChristopherCaradonna

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@mpraprost @eringold @ChristopherCaradonna @wenyikuang @mdahlhausen @JanghyunJK @LandanT @rHorsey