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

Upload precomputed buildstock.csv for national_baseline and testing_baseline for reproducing issue #1243

Closed
wants to merge 2 commits into from

Conversation

jmarrec
Copy link
Contributor

@jmarrec jmarrec commented May 15, 2024

Pull Request Description

Patches #1242

View changes without withespace https://github.com/NREL/resstock/pull/1243/files?w=1

Checklist

Not all may apply:

  • Tests (and test files) have been updated
  • Documentation has been updated
  • Changelog has been updated
  • openstudio tasks.rb update_measures has been run
  • No unexpected regression test changes on CI (checked comparison artifacts)

bundle exec rake workflow:analysis_tests

- name: Upload precomputed buildstocks
if: ${{ always() }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even if it failed, still upload the precomputed buildstocks. So that locally you can just reproduce with the same sampling.

This is helpful only if the sampling is non-deterministic.

Comment on lines +128 to +129
testing_baseline/buildstock.csv
national_baseline/buildstock.csv
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add these two too

name: precomputed_buildstocks

- name: Upload run_analysis.rb results
if: ${{ always() }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Always upload whatever results even if failed

@joseph-robertson
Copy link
Contributor

Thanks @jmarrec for testing/fixing this! I verified in #1240 that this is no longer an issue. On the changes around precomputed buildstocks, those are used for tests that are separate from test_testing_upgrades and test_national_upgrades (for these tests we use deterministic sampling).

@jmarrec jmarrec deleted the test-os-5194 branch June 3, 2024 21:56
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.

2 participants