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

Fixed the README settings parameter datatype with more clarity, which can otherwise be misleading especially to beginners. #120

Merged
merged 1 commit into from
Jul 12, 2022

Conversation

sambuddhac
Copy link
Contributor

Modified the dual_steepest_edge_weight_log_error_threshol, dual_simpl…ex_cost_perturbation_multiplie, and primal_simplex_bound_perturbation_multiplier values to double instead of Int (by simply inserting decimal points), since that's the required type and solution_file, log_file, and write_model_file values to string type ("") instead of blank. Otherwise, these give rise to errors while executing the HiGHS solver from JuMP.

…ex_cost_perturbation_multiplie, and primal_simplex_bound_perturbation_multiplier values to double instead of Int, since that's the required type and solution_file, log_file, and write_model_file values to string type instead of blank
@odow
Copy link
Member

odow commented Jul 9, 2022

How does this fix #118?

@odow
Copy link
Member

odow commented Jul 9, 2022

The options are pulled automatically from the upstream solver, so I guess we should also fix those upstream.

@codecov
Copy link

codecov bot commented Jul 9, 2022

Codecov Report

Merging #120 (88ff79e) into master (545245e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #120   +/-   ##
=======================================
  Coverage   83.35%   83.35%           
=======================================
  Files           3        3           
  Lines        1322     1322           
=======================================
  Hits         1102     1102           
  Misses        220      220           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 545245e...88ff79e. Read the comment docs.

@sambuddhac
Copy link
Contributor Author

sambuddhac commented Jul 9, 2022

The way it addresses issue #118 is that, without those "", the blank spaces are considered of type Nothing. Someone using it for the first time (like me till yesterday) may not be aware about this. If you now look into my fork that I shared with you, I have updated the HiGHS settings YAML files as well as the configure_highs.jl files with the changes I mentioned in this PR and this way things work smotth. The reason I was getting the error yesterday was because, as mentioned in the README, I was using blank spaces for those three settings parameters. Also the others in which I included the decimal points are supposed to be double. Without the decimal points, these also report error. Someone copying these settings from the README might be unaware. This also maintains the integrity and correctness of the documentation. Hope this explains. Please let me know if you have further questions. Thanks !!!!

@odow
Copy link
Member

odow commented Jul 12, 2022

the HiGHS settings YAML files

To clarify, the settings in the README are not in YAML format. It's just an ad-hoc text format used by the upstream HiGHS developers.

@odow odow merged commit 046c237 into jump-dev:master Jul 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants