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

Remove graph dependency from src/constraints/consumer.jl #944

Merged
merged 4 commits into from
Dec 18, 2024

Conversation

abelsiqueira
Copy link
Member

Part of #942
Closes #943

@abelsiqueira abelsiqueira force-pushed the 943-consumer-constraints branch from 97fc644 to 6206bbf Compare December 4, 2024 13:13
@abelsiqueira abelsiqueira added the benchmark PR only - Run benchmark on PR label Dec 4, 2024
@abelsiqueira abelsiqueira marked this pull request as ready for review December 4, 2024 13:13
Copy link
Contributor

github-actions bot commented Dec 4, 2024

Benchmark Results

b34be57... 26a9cd9... b34be57.../26a9cd91e9004a...
energy_problem/create_model 41.3 ± 1 s 43.9 ± 1.6 s 0.941
energy_problem/input_and_constructor 15.3 ± 0.24 s 18 ± 0.19 s 0.851
time_to_load 4.2 ± 0.015 s 4.27 ± 0.03 s 0.983
b34be57... 26a9cd9... b34be57.../26a9cd91e9004a...
energy_problem/create_model 0.614 G allocs: 21.3 GB 0.61 G allocs: 21.3 GB 1
energy_problem/input_and_constructor 0.0436 G allocs: 1.71 GB 0.0532 G allocs: 1.98 GB 0.866
time_to_load 0.159 k allocs: 11.2 kB 0.159 k allocs: 11.2 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).

Copy link

codecov bot commented Dec 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.88%. Comparing base (b34be57) to head (26a9cd9).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #944      +/-   ##
==========================================
- Coverage   94.95%   94.88%   -0.07%     
==========================================
  Files          29       29              
  Lines        1070     1075       +5     
==========================================
+ Hits         1016     1020       +4     
- Misses         54       55       +1     

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

@abelsiqueira abelsiqueira marked this pull request as draft December 4, 2024 15:45
@abelsiqueira abelsiqueira force-pushed the 943-consumer-constraints branch from 6206bbf to 0dc168c Compare December 5, 2024 12:23
@abelsiqueira abelsiqueira marked this pull request as ready for review December 5, 2024 12:58
@abelsiqueira abelsiqueira requested a review from datejada December 5, 2024 12:59
@abelsiqueira
Copy link
Member Author

@datejada, I've updated this to follow the new strategy. The code is still slow, though, so we might want to wait until all rep_periods_profiles are removed to see if it's possible to make it better.

@abelsiqueira abelsiqueira marked this pull request as draft December 6, 2024 09:49
@abelsiqueira abelsiqueira force-pushed the 943-consumer-constraints branch 2 times, most recently from f6e89c8 to e9ff4c1 Compare December 9, 2024 08:53
@abelsiqueira abelsiqueira force-pushed the 943-consumer-constraints branch 3 times, most recently from 0792027 to f15890d Compare December 16, 2024 19:51
@abelsiqueira abelsiqueira force-pushed the 943-consumer-constraints branch from f15890d to c97bdcd Compare December 18, 2024 10:11
@abelsiqueira
Copy link
Member Author

abelsiqueira commented Dec 18, 2024

@datejada and I evaluated the 3 options with DuckDB 1.1.2 locally (1-Using graph, 2-Using full DuckDB, which is the version from 2 weeks ago, and 3-Using the new profiles object). Here are the results:

Option 1 - Using graph[a].profiles_rep_periods

(1/1) benchmarking "energy_problem"...
  (1/2) benchmarking "input_and_constructor"...
  done (took 29.587801709 seconds)
  (2/2) benchmarking "create_model"...
  done (took 139.582342084 seconds)
done (took 172.174956 seconds)
1-element BenchmarkTools.BenchmarkGroup:
  tags: []
  "energy_problem" => 2-element BenchmarkTools.BenchmarkGroup:
          tags: []
          "input_and_constructor" => Trial(6.525 s)
          "create_model" => Trial(33.750 s)


Option 2 - Remove graph[a].profiles_rep_periods and compute aggregation with DuckDB

(1/1) benchmarking "energy_problem"...
  (1/2) benchmarking "input_and_constructor"...
  done (took 28.673210792 seconds)
  (2/2) benchmarking "create_model"...
  done (took 144.815846917 seconds)
done (took 175.86473875 seconds)
1-element BenchmarkTools.BenchmarkGroup:
  tags: []
  "energy_problem" => 2-element BenchmarkTools.BenchmarkGroup:
          tags: []
          "input_and_constructor" => Trial(6.508 s)
          "create_model" => Trial(34.858 s)


Option 3 - Change graph[a].profiles_rep_periods[...] to get profile_name with DuckDB and create profiles structure

(1/1) benchmarking "energy_problem"...
  (1/2) benchmarking "input_and_constructor"...
  done (took 34.309520792 seconds)
  (2/2) benchmarking "create_model"...
  done (took 127.688325292 seconds)
done (took 164.306446625 seconds)
1-element BenchmarkTools.BenchmarkGroup:
  tags: []
  "energy_problem" => 2-element BenchmarkTools.BenchmarkGroup:
          tags: []
          "input_and_constructor" => Trial(7.747 s)
          "create_model" => Trial(30.999 s)

Option 3 was a bit faster, and should be reasonable to maintain, so we're using it for now

@abelsiqueira
Copy link
Member Author

For future reference, this is the DuckDB query to join and aggregate the profiles (option 2):

         SELECT
             cons.*,
             ANY_VALUE(asset.type) AS type,
             ANY_VALUE(asset.consumer_balance_sense) AS consumer_balance_sense,
             ANY_VALUE(asset_milestone.peak_demand) AS peak_demand,
             COALESCE(MEAN(profile.value), 1) AS demand_agg
         FROM $cons AS cons
         LEFT JOIN asset
             ON cons.asset = asset.asset
         LEFT JOIN asset_milestone
             ON cons.asset = asset_milestone.asset
             AND cons.year = asset_milestone.milestone_year
         LEFT JOIN assets_profiles
             ON cons.asset = assets_profiles.asset
             AND cons.year = assets_profiles.commission_year
             AND assets_profiles.profile_type = 'demand'
         LEFT JOIN profiles_rep_periods AS profile
             ON profile.profile_name = assets_profiles.profile_name
             AND cons.year = profile.year
             AND cons.rep_period = profile.rep_period
             AND cons.time_block_start <= profile.timestep
             AND profile.timestep <= cons.time_block_end
         GROUP BY cons.*
         ORDER BY cons.index -- order is important
         

Copy link
Member

@datejada datejada left a comment

Choose a reason for hiding this comment

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

Reviewed during an online meeting

@abelsiqueira abelsiqueira marked this pull request as ready for review December 18, 2024 12:56
@abelsiqueira abelsiqueira merged commit 40a7436 into main Dec 18, 2024
5 of 7 checks passed
@abelsiqueira abelsiqueira deleted the 943-consumer-constraints branch December 18, 2024 13:13
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.

Don't get data from graph object in src/constraints/consumer.jl
2 participants