-
Notifications
You must be signed in to change notification settings - Fork 1
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
Refactor GenerationStandard #118
Conversation
Benchmark resultJudge resultBenchmark Report for /home/runner/work/E4ST.jl/E4ST.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/E4ST.jl/E4ST.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/E4ST.jl/E4ST.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #118 +/- ##
==========================================
+ Coverage 87.27% 87.38% +0.10%
==========================================
Files 36 36
Lines 2665 2679 +14
==========================================
+ Hits 2326 2341 +15
+ Misses 339 338 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for this! I added a couple questions but otherwise good to merge
year = years[yr_idx] | ||
haskey(targets, year) || continue | ||
curr_tgt_load_expr = tgt_load_expr[yr_idx] | ||
for bus_idx in bus_idxs, hr_idx in 1:nhour |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there an advantage to using a for loop like this instead of just sum() over the buses and hours? just curious
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's a minor performance thing - when calling sum
, JuMP constructs a new AffExpr object, then adds it to the expression. Now that you mention it, this is still creating an expression, since the multiplication is happening outside the add_to_expression!
call. I will update that.
Benchmark resultJudge resultBenchmark Report for /home/runner/work/E4ST.jl/E4ST.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/E4ST.jl/E4ST.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/E4ST.jl/E4ST.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
@sallyrobson this should be ready to go for your review.