For rootComplexity eval is called even if in check about stockfish HOT 5 CLOSED

I find Takes 1 and 3 to be rather illogical.

Take 1 results in a base time factor at the minimum of 0.85, changing the TM when in check, which is unnatural but might pass nonregression; Take 3 is unnecessarily complicated, appearing to be Take 2 bug fix plus a separate logic change intended to be a gainer.

Take 2, on the other hand, is the logical and natural fix: keep the current effect as is when not in check, and when in check, simply set the time factor to the non-effect value of 1.0. This is also more or less what I wrote and submitted. On this basis, I paused my test as duplicate of your Take 2. (In fact, your Take 2 is actually better than mine, since it fixes a second problem in master where we don't clear the mainThread complexity in ucinewgame.)

I recommend also pausing Take 1. Take 3 is interesting, I suppose, but in general I would prefer to keep bugfixes and gainers in separate patches and separate tests.

from stockfish.

On a deeper note, this suggests that the current default bench does not include any classical in check positions, i.e. doesn't offer sufficient code coverage from a test perspective. I suggest that, in addition to fixing this immediate issue, we upgrade the bench to include this check in the future.

from stockfish.

4515 LGTM (presuming passer).

On the question of preventing this in the future, we could certainly make that in check position be HCE in the default mixed bench, but is it perhaps more useful to instead add the same assert to nnue eval as well? Or is that better left alone?

from stockfish.

I'll add the additional assert in evaluate.

from stockfish.

I was just about to PR this lol master...dubslow:Stockfish:inCheckAssert

from stockfish.