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

Create all variables directly from connection and independent of dataframes #926

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

abelsiqueira
Copy link
Member

@abelsiqueira abelsiqueira commented Nov 9, 2024

Blocked by #925

Create all variables with connection instead of dataframes variable.

With the lookup created in #925, this allow us to

Close #923

Copy link

codecov bot commented Nov 9, 2024

Codecov Report

Attention: Patch coverage is 97.75281% with 2 lines in your changes missing coverage. Please review.

Project coverage is 96.17%. Comparing base (38c20d4) to head (d173c99).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/tmp.jl 96.29% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #926      +/-   ##
==========================================
- Coverage   98.76%   96.17%   -2.59%     
==========================================
  Files          30       30              
  Lines        1210     1202       -8     
==========================================
- Hits         1195     1156      -39     
- Misses         15       46      +31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@abelsiqueira abelsiqueira added the benchmark PR only - Run benchmark on PR label Nov 9, 2024
Copy link
Contributor

github-actions bot commented Nov 9, 2024

Benchmark Results

38c20d4... d173c99... 38c20d4.../d173c99d6d5f53...
energy_problem/create_model 44.8 ± 1.9 s 45.9 ± 0.77 s 0.975
energy_problem/input_and_constructor 9.94 ± 0.65 s 13.8 ± 0.19 s 0.721
time_to_load 4.09 ± 0.0034 s 4.1 ± 0.02 s 0.997
38c20d4... d173c99... 38c20d4.../d173c99d6d5f53...
energy_problem/create_model 0.66 G allocs: 22.6 GB 0.686 G allocs: 22 GB 1.02
energy_problem/input_and_constructor 0.0327 G allocs: 1.38 GB 0.0691 G allocs: 2.61 GB 0.528
time_to_load 0.157 k allocs: 11.1 kB 0.157 k allocs: 11.1 kB 1

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

@abelsiqueira abelsiqueira force-pushed the 923-variable-refactor-wip branch 2 times, most recently from f52d260 to fcf9833 Compare November 14, 2024 10:06
@abelsiqueira abelsiqueira changed the title 923 variable refactor wip Create all variables directly from connection and independent of dataframes Nov 14, 2024
Comment on lines +249 to +283
# The logic:
# - t_union has groups in order, then s:e ordered by s increasing and e decreasing
# - in a group (asset, year, rep_period) we can take the first range then
# - continue in the sequence selecting the next largest e
Copy link
Member Author

Choose a reason for hiding this comment

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

Remove or update

Copy link
Member Author

Choose a reason for hiding this comment

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

Use

  • 1:4 5:8 9:12
  • 1:3 4:4 5:9 10:12

1 2 3 4
o o o o
o o o
o o o o
o o o o
o o o o o
o o o o
o o o

o o o o
o

Copy link
Member Author

Choose a reason for hiding this comment

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

@suvayu check here

Separate creation of union and lowest resolution tables from
storage_level_intra_rp creation.

Closes #923
Copy link
Member

@gnawin gnawin left a comment

Choose a reason for hiding this comment

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

LGTM

@gnawin gnawin marked this pull request as ready for review November 25, 2024 16:23
@gnawin gnawin merged commit 127db68 into main Nov 25, 2024
6 of 7 checks passed
@abelsiqueira abelsiqueira deleted the 923-variable-refactor-wip branch November 25, 2024 17:55
@abelsiqueira
Copy link
Member Author

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark PR only - Run benchmark on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor variables
2 participants