Skip to content
Permalink
Browse files

Remove PieceType dimension. Bench: 3053669

  • Loading branch information...
31m059 committed Jan 8, 2019
1 parent f69106f commit b3b5730fafc5b2d0ef0e96616e011924c8c9efb2
Showing with 6 additions and 9 deletions.
  1. +6 −9 src/evaluate.cpp
@@ -117,13 +117,10 @@ namespace {
S(106,184), S(109,191), S(113,206), S(116,212) }
};

// Outpost[knight/bishop][supported by pawn] contains bonuses for minor
// pieces if they occupy or can reach an outpost square, bigger if that
// square is supported by a pawn.
constexpr Score Outpost[][2] = {
{ S(22, 6), S(36,12) }, // Knight
{ S( 9, 2), S(15, 5) } // Bishop
};
// Outpost[supported by pawn] contains bonuses for minor pieces if they
// occupy or can reach an outpost square, bigger if that square is
// supported by a pawn.
constexpr Score Outpost[2] = { S(10, 3), S(16, 6) };

// RookOnFile[semiopen/open] contains bonuses for each rook when there is
// no (friendly) pawn on the rook file.
@@ -321,10 +318,10 @@ namespace {
// Bonus if piece is on an outpost square or can reach one
bb = OutpostRanks & ~pe->pawn_attacks_span(Them);
if (bb & s)
score += Outpost[Pt == BISHOP][bool(attackedBy[Us][PAWN] & s)] * 2;
score += Outpost[bool(attackedBy[Us][PAWN] & s)] * (1 + (Pt == KNIGHT)) * 2;

else if (bb &= b & ~pos.pieces(Us))
score += Outpost[Pt == BISHOP][bool(attackedBy[Us][PAWN] & bb)];
score += Outpost[bool(attackedBy[Us][PAWN] & bb)] * (1 + (Pt == KNIGHT));

// Knight and Bishop bonus for being right behind a pawn
if (shift<Down>(pos.pieces(PAWN)) & s)

12 comments on commit b3b5730

@joergoster

This comment has been minimized.

Copy link

replied Jan 8, 2019

Not sure getting rid of one dimension with 2 entries only is worth slightly more complicated and less readable code.

@31m059

This comment has been minimized.

Copy link
Owner Author

replied Jan 8, 2019

@joergoster I didn't think this was any less readable...how would you feel about this functionally equivalent version?

            if (bb & s)
                score += Outpost[bool(attackedBy[Us][PAWN] & s)] * (4 - Pt) * 2;

            else if (bb &= b & ~pos.pieces(Us))
                score += Outpost[bool(attackedBy[Us][PAWN] & bb)] * (4 - Pt);
@snicolet

This comment has been minimized.

Copy link

replied Jan 8, 2019

@31m059 Your version relies on an implementation detail: the exact values of BISHOP and KNIGHT in the PieceType enumeration.

The Stockfish style would be:

    if (bb & s)
        score += Outpost[bool(attackedBy[Us][PAWN] & s)] * (Pt == KNIGHT ? 4 : 2);

    else if (bb &= b & ~pos.pieces(Us))
        score += Outpost[bool(attackedBy[Us][PAWN] & bb)] * (Pt == KNIGHT ? 2 : 1);

Note that the conditionals would be resolved at compile time because Pt is a template parameter. Whether this is more readable or preferable over master is another subject, of course :-)

@31m059

This comment has been minimized.

Copy link
Owner Author

replied Jan 8, 2019

@snicolet Yes, thank you! Ternary operators are far more legible, but I had avoided them, because I didn't realize that they would be evaluated at compile time.

@snicolet

This comment has been minimized.

Copy link

replied Jan 8, 2019

I would favor formulas over arrays if using the formula avoids the array access completely (no memory access at all, meaning all calculations can be done in registers) or if using the formula would save a significant amount of memory (for bigger arrays, for instance).

Here none of these conditions is true, so it is a close call whether the patch version is really a worthwhile simplification over master.

@31m059

This comment has been minimized.

Copy link
Owner Author

replied Jan 8, 2019

@snicolet I agree it's a small change, but would still advocate that it is a simplification, because it halves the number of parameters in Outpost without lessening readability. It may not be a huge number of redundant parameters (4), but they are nonetheless redundant.

@Vizvezdenec

This comment has been minimized.

Copy link

replied Jan 8, 2019

for now I set LTC prio to -1 till discussion about it being simplification or not ends :)

@31m059

This comment has been minimized.

Copy link
Owner Author

replied Jan 8, 2019

@Vizvezdenec I am actually curious to hear your opinion as well, if you don't mind.

@Vizvezdenec

This comment has been minimized.

Copy link

replied Jan 8, 2019

For me this looks like a simplification.
Also to note - bishop outposts are almost useless in terms of elo (like sub 1 elo in all tests) so will probably get simplified away sooner or later. So having extra numbers just for them is kinda meh and I like this change for this reason also.

@Alayan-stk-2

This comment has been minimized.

Copy link

replied Jan 9, 2019

Efficient formulas are way better than big 8x8 arrays (some pawn related arrays are quite ugly and psqt-like), but here the arrays are really tiny, far from a tuning nightmare.

Also, the knight outpost bonus in the arrays are around 2.5x those of the bishop ; with the formula we can only get 2x or 3x, hence the averaging seen here. Very small integers in formula create flexibility issues.

And as far as simplicity goes, I feel like this is more moving complexity around.

This change do not seem worthwhile to me.

bishop outposts are almost useless in terms of elo (like sub 1 elo in all tests) so will probably get simplified away sooner or later

If they do bring consistently 0.5+ elo, then their code footprint is quite acceptable imho. I'd rather try to simplify away the queen PSQT before this. But it's hard to measure the exact impact when the difference is so small.

@31m059

This comment has been minimized.

Copy link
Owner Author

replied Jan 9, 2019

@Alayan-stk-2 Thank you for taking the time to leave feedback. To follow up, if this array is small enough that there is not value in making it smaller with an equation, should we unroll the * 2 in master and create a 2x2x2 array? Outpost[knight/bishop][supported by pawn][currently occupied/reachable]

I think there's merit to either approach, but I don't think the current half-equation, half-precomputed approach makes sense. It certainly looks very inconsistent.

@Alayan-stk-2

This comment has been minimized.

Copy link

replied Jan 9, 2019

I can't tell you what's more efficient when it comes to micro-optimization (I don't know), my post only addressed code complexity and flexibility for the change done in the patch.

Effectively, a 2x2x2 array would hold 2x2x2x2 values because of mg/eg ; so getting from 8 array values to 16 array values. Still manageable, but it starts to get a bit annoying at this point.

It could still be an interesting experiment to try it, and see if a tune produce any noticeable divergence from the current x2 formula for occupied/reachable ? For a 16 values tune, though, you need to control noise, so setting up the tune properly is important (or doing it in 2-3 steps. For example, one step with all 16 and boosted ck, then one for the bishop array and one for the knight array each with a medium ck.). Would it be worth the resources usage at fishtest, though ?

Please sign in to comment.
You can’t perform that action at this time.