Modify investment limit to integer when necessary

[gnawin]

Pull request details

Describe the changes made in this pull request

Add floor function to investment limit when investment is integer, otherwise it stays the same.

List of related issues or pull requests

Closes #577

Collaboration confirmation

As a contributor I confirm

• I read and followed the instructions in README.dev.md
• The documentation is up to date with the changes introduced in this Pull Request (or NA)
• Tests are passing
• Lint is passing

[github-actions]

Benchmark Results

	b940b76...	d5ce24a...	b940b76.../d5ce24a5f25980...
energy_problem/create_model	0.0442 ± 0.00093 h	0.0434 ± 0.00083 h	1.02
energy_problem/input_and_constructor	12 ± 0.011 s	12 ± 0.023 s	0.999
time_to_load	2.5 ± 0.023 s	2.5 ± 0.016 s	1

	b940b76...	d5ce24a...	b940b76.../d5ce24a5f25980...
energy_problem/create_model	0.391 G allocs: 16 GB	0.391 G allocs: 16 GB	1
energy_problem/input_and_constructor	21 M allocs: 0.617 GB	21 M allocs: 0.617 GB	1
time_to_load	0.153 k allocs: 14.5 kB	0.153 k allocs: 14.5 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).

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (ace372f) to head (d5ce24a).
Report is 5 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #582   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           15        15           
  Lines          628       636    +8     
=========================================
+ Hits           628       636    +8     

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

[datejada]

It seems that the benchmark runs correctly TulipaEnergyModel, but then, processing the results, there is an error (a problem with an Inf in JSON); see the screenshot below. I'm not sure how to solve it. So, I suggest waiting to @abelsiqueira return from holiday to help us with this:

[gnawin]

The benchmark results show that the extra conditions slightly increase the time for energy_problem/create_model and energy_problem/input_and_constructor, and decreases it for time_to_load. I don't really know the difference between these, so I'll leave it for someone who knows to check and review.

[abelsiqueira]

Thanks for the PR, here is a short comment.

[gnawin]

@abelsiqueira thanks for the review! Based on your review, I added a function to calculate the bounds, how does it look now?

For me to learn 😄, an extra question is that, instead of using if-else, you suggest to set an default value first and then check for "outliers", is this a better way of programming in general, or is it due to performance concerns or some other reasons?

[abelsiqueira]

For me to learn 😄, an extra question is that, instead of using if-else, you suggest to set an default value first and then check for "outliers", is this a better way of programming in general, or is it due to performance concerns or some other reasons?

Nothing general in this case. Here I was trying to avoid a long expression from appearing twice, so it looks easier to read.
No performance implications happen either. Also, this was only recommended because the type of the value did not change. If, instead, we had value = round(Int, value), then the type would change to Int, which is bad in Julia.

If the expression was shorter, or involved different enough terms, I would have suggested a single-line solution using the ternary operator: value = cond ? a : b.

The change also reads fine in both situations. Originally, it was "if the investment is not integer, then its bound is A over B, otherwise, then the bound is the rounded value of A over B". Now it reads as "The bound is A over B. If the investment is integer, the round the bound".

[abelsiqueira]

Thanks!