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

Implement solution saving and exporting for refactored code #1000

Merged
merged 4 commits into from
Jan 20, 2025
Merged

Conversation

abelsiqueira
Copy link
Member

@abelsiqueira abelsiqueira commented Jan 15, 2025

  • Update function to compute dual
  • Create function to save solution and dual values to DuckDB connection
  • Update save_solution_to_file
  • Update tests

Related issues

Closes #115
Closes #818

Checklist

  • I am following the contributing guidelines

  • Tests are passing

  • Lint workflow is passing

  • Docs were updated and workflow is passing

@abelsiqueira abelsiqueira added the benchmark PR only - Run benchmark on PR label Jan 15, 2025
Copy link
Contributor

github-actions bot commented Jan 15, 2025

Benchmark Results

8322b5c... 838744f... 8322b5c.../838744f3abc16f...
energy_problem/create_model 31 ± 2.6 s 31.8 ± 1 s 0.977
energy_problem/input_and_constructor 16.7 ± 0.62 s 17.2 ± 0.41 s 0.973
time_to_load 4.05 ± 0.033 s 3.99 ± 0.059 s 1.02
8322b5c... 838744f... 8322b5c.../838744f3abc16f...
energy_problem/create_model 0.311 G allocs: 15.6 GB 0.311 G allocs: 15.6 GB 1
energy_problem/input_and_constructor 0.0532 G allocs: 1.98 GB 0.0531 G allocs: 1.97 GB 1
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).

@abelsiqueira abelsiqueira marked this pull request as ready for review January 15, 2025 15:44
@abelsiqueira abelsiqueira requested a review from datejada January 15, 2025 15:44
Copy link

codecov bot commented Jan 15, 2025

Codecov Report

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

Project coverage is 95.22%. Comparing base (8322b5c) to head (838744f).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/solve-model.jl 95.23% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1000      +/-   ##
==========================================
- Coverage   95.43%   95.22%   -0.22%     
==========================================
  Files          29       29              
  Lines        1140     1151      +11     
==========================================
+ Hits         1088     1096       +8     
- Misses         52       55       +3     

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

src/solve-model.jl Outdated Show resolved Hide resolved
src/solve-model.jl Outdated Show resolved Hide resolved
src/io.jl Outdated
@@ -320,158 +320,52 @@ end
save_solution_to_file(output_folder, energy_problem)

Saves the solution from `energy_problem` in CSV files inside `output_file`.
Notice that this assumes that the solution has been computed by [`save_solution!`](@ref).
"""
function save_solution_to_file(output_folder, energy_problem::EnergyProblem)
Copy link
Member

Choose a reason for hiding this comment

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

To differentiate more from the save_solution function and account for future options to export to another format, I propose renaming this function as export_solution_to_csv_files

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.

Thanks for the PR @abelsiqueira; I have left some comments. Let me know what you think.

@abelsiqueira
Copy link
Member Author

Thanks for the review! I've updated the PR

@abelsiqueira abelsiqueira requested a review from datejada January 20, 2025 14:22
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.

Thanks for the changes! I don´t have extra comments.

@datejada datejada merged commit 602a229 into main Jan 20, 2025
6 of 7 checks passed
@datejada datejada deleted the output branch January 20, 2025 16:29
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
2 participants