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

Rhorsey/sampling v2 plotting patch #217

Merged
merged 4 commits into from
Sep 23, 2024
Merged

Conversation

rHorsey
Copy link
Collaborator

@rHorsey rHorsey commented Sep 18, 2024

Pull request overview

Updates plotting scripts for comparing upgrades, baselines, and CBECS. AMI and Timeseries are up next - EIA requires a transition to our current postproc paradigm.

  • Fixes Postprocessing errors #216 although I'm pretty sure the baseline there isn't correct, thus I've added a 10k sample to sampling/resources.

Pull Request Author

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.

@rHorsey rHorsey added sampling PR improves or adds to the sampling methodology postprocessing PR improves or adds postprocessing content Pull Request - Ready for CI labels Sep 18, 2024
@rHorsey rHorsey changed the base branch from develop to main September 18, 2024 01:36

def add_weights_aportioned_by_stock_estimate(self, apportionment: Apportion, keep_n_per_apportionment_group=False):
# This function doesn't support already CBECS-weighted self.data - error out
if self.CBECS_WEIGHTS_APPLIED:
Copy link
Collaborator

Choose a reason for hiding this comment

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

GREAT!

@@ -2478,7 +2548,8 @@ def rmv_units(c):
assert new.startswith(old)
logger.debug(f'{old} -> {new}')

self.data = self.data.rename(crnms)
lazyframe = lazyframe.rename(crnms)
return lazyframe
Copy link
Collaborator

Choose a reason for hiding this comment

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

return lazyframe.rename(crnms) will be exctaly same.

wenyikuang
wenyikuang previously approved these changes Sep 18, 2024
Copy link
Collaborator

@wenyikuang wenyikuang left a comment

Choose a reason for hiding this comment

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

Looks good, but I tried to run comstock_to_cbecs_comparison.py with the default template but not works due to eulp/euss_com/cycle_4_sampling_test_rand_985932_20240321/cycle_4_sampling_test_rand_985932_20240321/ is not existed, the right path should be cycle_4_sampling_test_rand_985932_20240321/rand_985932_20240321/baseline/

@rHorsey
Copy link
Collaborator Author

rHorsey commented Sep 19, 2024

Thanks @wenyikuang - updated the paths and also switched to using Resource instead of Client in boto3 to avoid an error with the policy on stuff uploaded from Kestrel using the default upload policy.

@rHorsey
Copy link
Collaborator Author

rHorsey commented Sep 19, 2024

@ChristopherCaradonna can you delete the lighting upgrade in you comstock_data and try rerunning compare_upgrades again? If that works as expected can you review / approve please?

Copy link
Contributor

@ChristopherCaradonna ChristopherCaradonna left a comment

Choose a reason for hiding this comment

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

@rHorsey Postprocessing completed, most plots look good.

2 Issues:

  1. Census Division plots show no energy / square footage in "Middle Atlantic".
  2. Non-breaking error message, as show below
    Bonus) I get an error for not having 'import json' in the s3_utilties_mixin.py

--- Logging error ---
Traceback (most recent call last):
File "C:\Users\ccaradon\AppData\Local\anaconda3\envs\comstockpostproc\lib\logging_init_.py", line 1083, in emit
msg = self.format(record)
File "C:\Users\ccaradon\AppData\Local\anaconda3\envs\comstockpostproc\lib\logging_init_.py", line 927, in format
return fmt.format(record)
File "C:\Users\ccaradon\AppData\Local\anaconda3\envs\comstockpostproc\lib\logging_init_.py", line 663, in format
record.message = record.getMessage()
File "C:\Users\ccaradon\AppData\Local\anaconda3\envs\comstockpostproc\lib\logging_init_.py", line 367, in getMessage
msg = msg % self.args
TypeError: not all arguments converted during string formatting
Call stack:
File "C:\Users\ccaradon\Documents\GitHub\ComStock\postprocessing\compare_upgrades.py", line 73, in
main()
File "C:\Users\ccaradon\Documents\GitHub\ComStock\postprocessing\compare_upgrades.py", line 58, in main
comstock.add_national_scaling_weights(cbecs, remove_non_comstock_bldg_types_from_cbecs=True)
File "C:\Users\ccaradon\Documents\GitHub\ComStock\postprocessing\comstockpostproc\comstock.py", line 1900, in add_national_scaling_weights
logger.info("sf wt_area_col shape: ", sf[wt_area_col].shape)
Message: 'sf wt_area_col shape: '
Arguments: ((14,),)

# comstock.create_geospatially_resolved_aggregations(comstock.STATE_ID, pretty_geo_col_name='state_id')
# comstock.create_geospatially_resolved_aggregations(comstock.COUNTY_ID, pretty_geo_col_name='county_id')
# TODO Long is def not working as expected anymore...
# comstock.export_to_csv_long() # Long format useful for stacking end uses and fuels
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is ComStock wide export?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's the create_national_aggregations method now - same code under the hood but redone to get correctly weighted national level results!

@rHorsey
Copy link
Collaborator Author

rHorsey commented Sep 19, 2024

@ChristopherCaradonna 's issues addressed - ready for approval! Great catch on mid-atlantic - many thanks!.

Copy link
Contributor

@ChristopherCaradonna ChristopherCaradonna left a comment

Choose a reason for hiding this comment

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

Changes look good, approved.

@rHorsey rHorsey merged commit 2a6b506 into main Sep 23, 2024
0 of 3 checks passed
@rHorsey rHorsey deleted the rhorsey/sampling_v2_plotting_patch branch September 23, 2024 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
postprocessing PR improves or adds postprocessing content Pull Request - Ready for CI sampling PR improves or adds to the sampling methodology
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Postprocessing errors
3 participants