Don't print debug actions when k=-1

[JoshuaLampert]

This fixes a small bug, when using some debug actions (DebugTime, DebugEntryChange, DebugWarnIfCostNotFinite, DebugWarnIfFieldNotFinite, and DebugWarnIfGradientNormTooLarge) together with DebugEvery (with always_update = true), where the actions ignore the DebugEvery and print their output at each iteration instead of only every every iterations. This is because they don't add a guard for k = -1, which is passed by DebugEvery for the iterations that should be skipped. For the warnings it might be debatable whether they should intentionally skip the DebugEvery or not, but since DebugWarnIfCostIncreases and DebugWarnIfStepsizeCollapsed already had the k < 0 included, I also added it to the other three warning debug actions for consistency. I can also remove it consistently for all five warning debug actions if preferred.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (9489fd8) to head (b8f71a0).

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #609   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           91        91           
  Lines        10054     10054           
=========================================
  Hits         10054     10054           

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

[JoshuaLampert]

Failing tests with Julia nightly seem unrelated and an issue of HSL.jl.

[kellertuer]

For the “normal” debugs I agree, the idea would be to never print with negative k, the implementation was not consistent there – thanks for spotting and making it consistent!

For the Warnings, you are right that was also not consistent. but I am just not sure which mode is the better one: If one activates / adds a warning I think always warning would be better?
Otherwise a user might say “I ask the algorithm to warn me and it did not.”

So we could also remove the k < 0 checks from the two that currently have it – instead. I think this has until now really just not been fixed / documented, which warning mode is the better one.

[kellertuer]

...and yes, I have not yet understood the nightly errors, but they are there for a while already and not your fault. I should maybe check whom to ask on slack somewhen.

[JoshuaLampert]

For the Warnings, you are right that was also not consistent. but I am just not sure which mode is the better one: If one activates / adds a warning I think always warning would be better?
Otherwise a user might say “I ask the algorithm to warn me and it did not.”

Yes, I mostly agree with that. But you could also say "I ask the algorithm to only give debug output every so and so iterations via DebugEvery and it did not" :)
But I'll remove the k < 0 from all warnings.

[kellertuer]

Sure, as I wrote, both sides have their good arguments.

This looks good now. Could you specify the two aspects we unified the actions to in the changes as well?

Then this should be good to go and I can register it after merge.

[JoshuaLampert]

Done in 35dc2c5.

[kellertuer]

Nice. As soon as the tests pass, I can merge this and register the new version for you.

Thanks for spotting this and unifying the behaviour.