Get correct highs iteration count depending on method

[AlexGisi]

Fixes and Summary/Motivation # .

In the highs solver interface, instead of always reporting the simplex iteration count, we should get the iteration count of the method that was used. See #3753.

Changes proposed in this PR:

• Take the max of the available iteration counts, if the info is valid

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

1. I agree my contributions are submitted under the BSD license.
2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

[jsiirola]

Can you add a test that exercises one (or a couple) different solvers?

[AlexGisi]

Can you add a test that exercises one (or a couple) different solvers?

@jsiirola Is this what you were thinking: ea5ba4f

I added internal assertions so those will catch any oddities

[jsiirola]

Shouldn't the iteration count be strictly greater than 0 in this case?

[mrmundt]

Okay, I was playing with the tests and changed the MIP demo to this:

    def test_mip_demo(self):
        # Build MIP
        from pyomo.core.tests.examples.pmedian_concrete import create_model

        M = create_model()

        solver = Highs()
        results = solver.solve(M)
        self.assertEqual(results.solution_status, SolutionStatus.optimal)
        self.assertTrue(results.iteration_count > 0)

Lo-and-behold, this causes an assertion error:

                positive_iters = [c for c in counts if c > 0]
                if not positive_iters:
                    assert any(
                        (c == 0 for c in counts)
                    ), "At least one iteration count should have a non-negative value"
                    results.iteration_count = 0
                else:
                    assert (
>                       len(positive_iters) == 1
                    ), "Only one iteration count should have a positive value"
E                   AssertionError: Only one iteration count should have a positive value

pyomo/contrib/solver/solvers/highs.py:773: AssertionError

simplex_iteration_count and mip_node_count are both positive values (in this case, 5 and 1). So we need to reconsider this PR / how we actually want to fix the problem since iteration_count in general is not well defined for MIP.

@mrmundt discovered an issue so this PR needs more work and will need a new review

[AlexGisi]

I suggest the problem raised by @mrmundt should be addressed by removing mip_node_count from the list of considered counts. It's a count of nodes, not solver iterations in the sense of the other methods. If we want to report the MIP node count, perhaps there would be a separate field in the results class. Since during MIP solve the other counts are populated corresponding to the method used within nodes (by default simplex) the count selection logic holds.

[jsiirola]

This PR opened up a whole side discussion. Talking with @bknueven (who happened to be sitting next to me), I think we came to the conclusion that we should argue for removing iteration_count from the "standard" set of results. Instead, iteration count(s) should be put in the solver-specific results. For MIP solvers, there really isn't a meaningful "iteration count" (@bknueven actually argued that "iteration count" may only be meaningful for interior point solvers ;) ), so trying to shoe-horn the concept into some kind of standard result would be misleading at best.

[mrmundt]

@AlexGisi - We talked about this in the dev call today, and it's going to require a mini-rework of all the different solvers to address iteration counts. I'm going to close this PR but will incorporate the changes from it into the new PR that we open to address the "bigger problem."