Implement logging for IIS computation

[haman80]

• Logging for elasticity and deletion filter iterations: Added iteration logging for the elasticity filter and deletion filter. Each log line reports the iteration number, number of rows, and runtime. The header is reprinted every 20 lines, and output is emitted every 5 seconds with the first and last iterations always printed.
• Fixed an issue in the elasticity filter where both elastic variables could be simultaneously enforced on a row; such rows are now correctly marked as boxed.
• Example display with iis_strategy = kIisStrategyIrreducible

[codecov]

Codecov Report

❌ Patch coverage is 89.00000% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.54%. Comparing base (ff49d13) to head (e3e7bf2).
⚠️ Report is 32 commits behind head on latest.

Files with missing lines	Patch %	Lines
highs/lp_data/HighsInterface.cpp	72.97%	10 Missing ⚠️
highs/lp_data/HighsIis.cpp	98.36%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##           latest    #2918   +/-   ##
=======================================
  Coverage   81.54%   81.54%           
=======================================
  Files         347      347           
  Lines       86865    86942   +77     
=======================================
+ Hits        70838    70901   +63     
- Misses      16027    16041   +14     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[fwesselm]

Looks great, @haman80. Just one general comment: You could use static_cast instead of C-style casts in your code.

[jajhall]

Looks great, @haman80. Just one general comment: You could use static_cast instead of C-style casts in your code.

You've commented before on the use of C-style casts in code I've written, and it's almost always when using int formatting in printf for identifiers that are HighsInt, or size_t.

I've just looked up reasons not to use C-style casts, and see that there are four C++ casts that it can mask - depending on the argument? I don't understand these, but what was clear is that int(i) is never ambiguous. For me it's far more readable than static_cast(i), to say nothing of the speed of typing.

So, if I'm correct in saying that int(i) is never ambiguous - and my extensive use of it without problem in HiGHS suggests that it is - then it comes down to the programmer's style.

Am I correct?

[jajhall]

I see that in a couple of places @haman80 is casting size_t to HighsInt, and then using HIGHSINT_FORMAT in printf. Although I was around when it was introduced, I find that it makes formatting very hard to read, which is why I cast to int. I know that, in theory, overflow can occur, but in HiGHS the only practical need for HighsInt is for start pointers in compressed column/row storage of sparse matrices, as the number of nonzeros may exceed 2,147,483,647 (Google wanted it so they could use our presolve). I think it's safe to assume that the number of rows and columns in a problem won't exceed this value - and I'm only talking about printf.

[fwesselm]

Looks great, @haman80. Just one general comment: You could use static_cast instead of C-style casts in your code.

You've commented before on the use of C-style casts in code I've written, and it's almost always when using int formatting in printf for identifiers that are HighsInt, or size_t.

I've just looked up reasons not to use C-style casts, and see that there are four C++ casts that it can mask - depending on the argument? I don't understand these, but what was clear is that int(i) is never ambiguous. For me it's far more readable than static_cast(i), to say nothing of the speed of typing.

So, if I'm correct in saying that int(i) is never ambiguous - and my extensive use of it without problem in HiGHS suggests that it is - then it comes down to the programmer's style.

Am I correct?

An excerpt from a discussion on stack overflow:

C-style cast and function-style cast are casts using (type)object or type(object), respectively, and are functionally equivalent. They are defined as the first of the following which succeeds:

const_cast
static_cast (though ignoring access restrictions)
static_cast (see above), then const_cast
reinterpret_cast
reinterpret_cast, then const_cast
It can therefore be used as a replacement for other casts in some instances, but can be extremely dangerous because of the ability to devolve into a reinterpret_cast, and the latter should be preferred when explicit casting is needed, unless you are sure static_cast will succeed or reinterpret_cast will fail.

C-style casts also ignore access control when performing a static_cast, which means that they have the ability to perform an operation that no other cast can.

I think that it is better to be explicit and use static_cast. In my opinion it also makes finding type conversions easier (just search for static_cast).

But, if this is controversial, I am also fine with keeping the code as it is.

[BenChampion]

It's not just a matter of style, and my €/£/$0.02 is that we should strongly prefer *_cast.

Just to restate what Franz said, this time using another answer from Stack Overflow

Answer to question "C++ cast syntax styles"

The constructor syntax (official name: function-style cast) is semantically the same as the C-style cast and should be avoided as well

It's best practice never to use C-style casts for three main reasons:

• as already mentioned, no checking is performed here. The programmer simply cannot know which of the various casts is used which weakens strong typing
• the new casts are intentionally visually striking. Since casts often reveal a weakness in the code, it's argued that making casts visible in the code is a good thing.
• this is especially true if searching for casts with an automated tool. Finding C-style casts reliably is nearly impossible.

[jajhall]

I totally agree with using '*_cast' if there's the least chance of misinterpretation or we need to search for the use cases. However, if I'm right in saying that int(i) is always interpreted correctly when i is HighsInt or size_t - and please correct me if it might not be - then it's a matter of personal style.

There's also an argument for only using '*_cast' when necessary so that searches aren't swamped by all the uses when printing HighsInt or size_t 😅

[haman80]

Hi all,
I have removed HIGHSINT_FORMAT from the IIS logging functions and replaced it with %d, casting to int where needed using the function-style cast. As Julian noted, this should be safe as long as the number of iterations and rows stays below INT_MAX. Please let me know if this looks good or if you have any other feedback.