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

Add options to improve model creation speed #1149

Merged
merged 8 commits into from
Dec 17, 2024

Conversation

datejada
Copy link
Member

@datejada datejada commented Dec 3, 2024

Following the performance tips from JuMP docs, this PR implements the following option in the run_spineopt function:

  • use_model_names(default = true): whether or not to create the model with names
  • add_bridges(default = false): whether or not bridges from JuMP to the solver should be added to the model

Notes

  • The use_model_names default is equal to true, so the users can gain more speed when creating the model by setting this option to false.

  • The add_bridges option is set to true for the MGA algorithm, since it is necessary to run that feature.

Fixes #1148

Checklist before merging

  • Documentation is up-to-date
  • Unit tests have been added/updated accordingly
  • Code has been formatted according to SpineOpt's style
  • Unit tests pass

@datejada
Copy link
Member Author

datejada commented Dec 3, 2024

These are the results from the benchmark.

Results with the name creation and bridges

 BenchmarkTools.Trial: 3 samples with 1 evaluation.
 Range (min … max):  67.082 s … 78.006 s  ┊ GC (min … max): 19.92% … 18.47%
 Time  (median):     71.319 s             ┊ GC (median):    18.73%
 Time  (mean ± σ):   72.135 s ±  5.508 s  ┊ GC (mean ± σ):  18.99% ±  0.78%

 Memory estimate: 44.96 GiB, allocs estimate: 714989451.

Results disabling only the name creation

 BenchmarkTools.Trial: 3 samples with 1 evaluation.
 Range (min … max):  60.068 s … 65.056 s  ┊ GC (min … max): 18.35% … 21.78%
 Time  (median):     64.747 s             ┊ GC (median):    18.89%
 Time  (mean ± σ):   63.290 s ±  2.795 s  ┊ GC (mean ± σ):  19.71% ±  1.85%

 Memory estimate: 44.01 GiB, allocs estimate: 691724795.

Results disabling the name creation and the bridges

BenchmarkTools.Trial: 3 samples with 1 evaluation.
 Range (min … max):  58.890 s … 61.703 s  ┊ GC (min … max): 18.55% … 19.59%
 Time  (median):     60.773 s             ┊ GC (median):    19.89%
 Time  (mean ± σ):   60.455 s ±  1.433 s  ┊ GC (mean ± σ):  19.44% ±  0.80%

Memory estimate: 44.01 GiB, allocs estimate: 691724494.

Copy link

codecov bot commented Dec 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.35%. Comparing base (6dfccfa) to head (27cbcff).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1149      +/-   ##
==========================================
+ Coverage   87.33%   87.35%   +0.01%     
==========================================
  Files         143      143              
  Lines        4130     4135       +5     
==========================================
+ Hits         3607     3612       +5     
  Misses        523      523              

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

@datejada datejada changed the title Add option to disable names Add options to improve speed on model creation Dec 3, 2024
@datejada datejada changed the title Add options to improve speed on model creation Add options to improve model creation speed Dec 3, 2024
@datejada datejada requested a review from DillonJ December 10, 2024 17:31
@datejada
Copy link
Member Author

@DillonJ, the changes are ready for your review. Please let me know your comments and suggestions. Thanks!

@datejada
Copy link
Member Author

Changes commented during developers meeting

@datejada datejada merged commit f6bf05a into master Dec 17, 2024
7 checks passed
@datejada datejada deleted the 1148-add-option-to-disable-names-in-the-model branch December 17, 2024 13:27
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.

Add option set_string_names_on_creation in run_spineopt
1 participant