Introduce two maxiter safeguards to WolfePowell and rewrite the whole messaging system

[kellertuer]

This resolves #495.
@jonas-pueschel is this what you had in mind? Note that I even adapted the message idea from Armijo, so one can even check afterwards whether these have happened and display a message.

📋

• check how to do code coverage - maybe by setting them to zero so they fire directly.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.86%. Comparing base (0f15e0c) to head (e32410b).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #531      +/-   ##
==========================================
+ Coverage   99.78%   99.86%   +0.08%     
==========================================
  Files          87       89       +2     
  Lines        9863     9932      +69     
==========================================
+ Hits         9842     9919      +77     
+ Misses         21       13       -8     

☔ 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.

[kellertuer]

So, here is my approach to a rework. It took me 3hours to come up with this, and there is still quite a few tests failing now. But before I fix them: Does this allocate less enough?

I think I am the worst person to ask about allocations, I am not good at this. But I also do not like spamming a strict with a million int fields. So before I try to fix the tests, some feedback on whether this is reasonable or I should abandon this PR would be nice. This is a total detour from what the actual PR is about, but well. I am also used to that by now. My code is so bad, that no PR ends up doing what it intends to do but also rewrites a lot of other parts. It is what it is.

[kellertuer]

Note also that the new approach provides less information that the previous approach. Since the string has to be generates long long long long after we left the loop where the message was originally generated, some of the information from within that loop no longer exists. But storing that somewhere allocates, so I was unsure that is allowed.

[mateuszbaran]

The PR was IMO fine before your 3 hour long rework and you didn't have to touch other line searches here so maybe you are trying to do too much in some pull requests.

Dict should allocate less than a String but it's still slower than any of my suggestions or a NamedTuple.

Note also that the new approach provides less information that the previous approach. Since the string has to be generates long long long long after we left the loop where the message was originally generated, some of the information from within that loop no longer exists. But storing that somewhere allocates, so I was unsure that is allowed.

Of course you can store it, I'd just prefer to have the values stored in something that is concretely typed and not a String.

[kellertuer]

Well before my 3 hour research we were approaching and infinite number of Int fields in all step sizes.

If the current one is not good, then I give up. That was the best I could do for something that is a total detour of this PR already.

Then I either leave this open until this is fixed in some way by someone else of slowly recommend to close. I am very tired of trying to fix code with a few lines that turns into a month of reworking a large code base – which is what I currently exclusively do.

[kellertuer]

A named tuple as immutable Ints as fields, so we can not use that here, since when we want to set a number to the current int, it would not work,

And from me using a Dict you can again recognise – I should not work on allocations. I thought dicts were relatively efficient, at least when keys and values are concretely typed and not large unions. But well...

[mateuszbaran]

A named tuple as immutable Ints as fields, so we can not use that here, since when we want to set a number to the current int, it would not work,

There are some ways to work around this but I'd have to investigate a bit what works fine and doesn't look terrible.

And from me using a Dict you can again recognise – I should not work on allocations. I thought dicts were relatively efficient, at least when keys and values are concretely typed and not large unions. But well...

Dict is relatively efficient, yes, it's just still slower than mutable struct or NamedTuple. For problems with >100 variables the difference would be negligible but for small ones that's not always the case.

[mateuszbaran]

Maybe let's finish this on Monday? Discussing this in person would make it easier to find the right solution.

[kellertuer]

Sure, I am out of ideas here anyways by now ;) The current state was the best I could come up with and I feel even that is clumsy when generating the actual message for a state.

[kellertuer]

Ah nice, my last commit fixed it and we even cover 8 lines more (13 uncovered instead of 21) now, maybe due to the extra cases in WolfePowell.

I would consider this finished then. Thanks for the discussions and help today :)

[mateuszbaran]

Cool! I think this is a really nice improvement.