Skip to content

Refactoring Highs::run()#2740

Merged
jajhall merged 19 commits intolatestfrom
fix-2738
Jan 12, 2026
Merged

Refactoring Highs::run()#2740
jajhall merged 19 commits intolatestfrom
fix-2738

Conversation

@jajhall
Copy link
Member

@jajhall jajhall commented Jan 11, 2026

Sufficient refactoring of Highs::run() has been done to enable #2635 to be fixed.

Split Highs::run() into a set of nested methods according to HighsRun.md, which includes chat about what still needs to be done. This will take time, so this PR avoids a long-lasting dev branch.

@jajhall
Copy link
Member Author

jajhall commented Jan 11, 2026

Closes #2738

Copy link
Collaborator

@Opt-Mucca Opt-Mucca left a comment

Choose a reason for hiding this comment

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

I'm a big fan of refactoring this! Some current thoughts:

  • I dislike calling optimizeHighs inside of runFromUserScaling. I'd rather a scale function and then an undoscale function on either side of the call.
  • Why do we actually need runFromExe? Could this not just be run?
  • I like the general breakdown (run-time options -> user scaling -> "mods")

@jajhall
Copy link
Member Author

jajhall commented Jan 12, 2026

I'm a big fan of refactoring this! Some current thoughts:

  • I dislike calling optimizeHighs inside of runFromUserScaling. I'd rather a scale function and then an undoscale function on either side of the call.

If I understand correctly, you're suggesting putting a scale function and then an undoscale function either side of optimizeHighs within runFromExe?

This does reduce the number of nested methods, and I can't think of circumstances where user scaling would need to be applied without the "Highs files" methods, so fair enough

  • Why do we actually need runFromExe? Could this not just be run?

I find run to be so opaque, but if we eliminate runFromUserScaling then it's the only run method, so there's no distinction to be made.

  • I like the general breakdown (run-time options -> user scaling -> "mods")

Thanks

@Opt-Mucca
Copy link
Collaborator

If I understand correctly, you're suggesting putting a scale function and then an undoscale function either side of optimizeHighs within runFromExe?

Yup!

@jajhall
Copy link
Member Author

jajhall commented Jan 12, 2026

Thanks, I've simplified the calling structure.

Are you OK with this PR?

Copy link
Collaborator

@Opt-Mucca Opt-Mucca left a comment

Choose a reason for hiding this comment

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

Happy with the change!

@jajhall jajhall merged commit 2f75610 into latest Jan 12, 2026
390 of 392 checks passed
@jajhall jajhall deleted the fix-2738 branch January 12, 2026 17:29
@jajhall jajhall mentioned this pull request Jan 12, 2026
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.

2 participants