position.cpp new possibilities

[lech]

1.
Too long code seems to be in the following function:

Code: Select all

bool Position::pl_move_is_evasion(Move m, Bitboard pinned) const
{
  assert(is_check());

  Color us = side_to_move();
  Square from = move_from(m);
  Square to = move_to(m);

  // King moves and en-passant captures are verified in pl_move_is_legal()
  if (type_of_piece_on(from) == KING || move_is_ep(m))
      return pl_move_is_legal(m, pinned);

  Bitboard target = checkers();
  Square checksq = pop_1st_bit(&target);

  if (target) // double check ?
      return false;

  // Our move must be a blocking evasion or a capture of the checking piece
  target = squares_between(checksq, king_square(us)) | checkers();
  return bit_is_set(target, to) && pl_move_is_legal(m, pinned);
}

possible simplifications:

Code: Select all

---
Bitboard target = checkers();
  if (target & (target - 1))  // double check ? // changed
      return false;

  if (move_is_castle(m)) // added
      return false;  // castle is prohibit when is check !

// Our move must be a blocking evasion or a capture of the checking piece
  target = squares_between(checksq, king_square(us)) | checkers();
  return bit_is_set(target, to) &&  (!pinned || !bit_is_set(pinned, from)); // changed

2.
The constructor data CheckInfo::CheckInfo(const Position& pos) are using by 2 permanet function:
bool Position::move_is_check(Move m, const CheckInfo& ci) and
void Position::do_move(Move m, StateInfo& newSt, const CheckInfo& ci, bool moveIsCheck)
It seems to be that this constructor makes many useless and time-consuming code.
My proposition is to remove it (or all the struct CheckInfo).

Code: Select all

CheckInfo::CheckInfo(const Position& pos) {

  Color us = pos.side_to_move();
  Color them = opposite_color(us);

  ksq = pos.king_square(them);
  dcCandidates = pos.discovered_check_candidates(us);

  checkSq[PAWN] = pos.attacks_from<PAWN>(ksq, them);
  checkSq[KNIGHT] = pos.attacks_from<KNIGHT>(ksq);
  checkSq[BISHOP] = pos.attacks_from<BISHOP>(ksq);
  checkSq[ROOK] = pos.attacks_from<ROOK>(ksq);
  checkSq[QUEEN] = checkSq[BISHOP] | checkSq[ROOK];
  checkSq[KING] = EmptyBoardBB;
}

And to change the two function:

Code: Select all

void Position::do_move(Move m, StateInfo& newSt, const CheckInfo& ci, bool moveIsCheck)
---
if (moveIsCheck)
  {
      if (ep | pm)
          st->checkersBB = attackers_to(king_square(them)) & pieces_of_color(us);
      else
      {

        // added.
       // note that now it works only if moveIsCheck!
         pt == PAWN ? ci.checkSq[PAWN] = pos.attacks_from<PAWN>(ci.ksq, them) : ci.checkSq[pt] = pos.attacks_from<pt>(ci.ksq);  
        ci.dcCandidates = pos.discovered_check_candidates(us);
        ci.ksq = pos.king_square(them);
       
      // Direct checks    
---

and:

Code: Select all

bool Position::move_is_check(Move m, const CheckInfo& ci) const {
---
  PieceType pt = type_of_piece_on(from);

  // added
  Color us = pos.side_to_move();
  Color them = opposite_color(us);
  pt == PAWN ? ci.checkSq[PAWN] = pos.attacks_from<PAWN>(ci.ksq, them) : ci.checkSq[pt] = pos.attacks_from<pt>(ci.ksq); 

 // Direct check ?
  if (bit_is_set(ci.checkSq[pt], to))
      return true;

  // added
  dcCandidates = pos.discovered_check_candidates(us);
  ci.ksq = pos.king_square(them);

  // Discovery check ?
---
 

Two other functions needs simple corrections too:
bool Position::has_mate_threat() and perft().
And Stockfish is no. 1

[lech]

Of course the second point is wrong (a joke)

[mcostalba]

Of course the second point is wrong (a joke)

How can you use 'checksq' in the last statements of point 1 if you have removed its definition in the upper part ?

[lech]

Of course the second point is wrong (a joke)

How can you use 'checksq' in the last statements of point 1 if you have removed its definition in the upper part ?

Yes, sorry it was not my happy day but I believe that today is better.
3.
A simplification:

Code: Select all

template<bool FindPinned>
Bitboard Position::hidden_checkers(Color c) const {
  
  if (FindPinned && pinners)
      pinners &= ~st->checkersBB;
  

why not?

Code: Select all

Bitboard pinners = pieces_of_color(FindPinned ? opposite_color(c) : c) & ~st->checkersBB;
 

Next, not for position.cpp.
4.
The status of half-open files matches to material events more than to pawn ones.
It seems to be advisable to move the next lines (and associated 3 functions) to mi-> .

Code: Select all

template<Color Us>
Score PawnInfoTable::evaluate_pawns(const Position& pos, Bitboard ourPawns,
                                    Bitboard theirPawns, PawnInfo* pi) const {

  // Initialize halfOpenFiles[]
  for (f = FILE_A; f <= FILE_H; f++)
      if (!(ourPawns & file_bb(f)))
          pi->halfOpenFiles[Us] |= (1 << f);

5.
The static evoluation of KRRPs/KRRPs endgames can cheat.
My proposition is to add a factor.
Note that it can cancel opposite bishops factor, and it seems to be no importance.
The old chess rule says that the weaker side should avoid exchanging of rooks.

Code: Select all

MaterialInfo* MaterialInfoTable::get_material_info(const Position& pos) 
---
if (pos.piece_count(WHITE, ROOK)  > 1  && pos.piece_count(BLACK, ROOK)  > 1))
{
Value sfx = Value(32 * 4 *  RookValueMidgame / int( pos.non_pawn_material(WHITE) + pos.non_pawn_material(BLACK))); 
mi->factor[WHITE] = mi->factor[BLACK] = uint8_t(SCALE_FACTOR_NORMAL - sfx);
}
// Compute the space weight

[lech]

I understend that 1 in this form is not better than the original code.

Code: Select all

/// Position::pl_move_is_evasion() tests whether a pseudo-legal move is a legal evasion

bool Position::pl_move_is_evasion(Move m, Bitboard pinned) const
{
  assert(is_check());

  Color us = side_to_move();
  Square from = move_from(m);
  Square to = move_to(m);

  // King moves and en-passant captures are verified in pl_move_is_legal()
  if (type_of_piece_on(from) == KING || move_is_ep(m))
      return pl_move_is_legal(m, pinned);

  Bitboard target = checkers();
  Square checksq = pop_1st_bit(&target);

  if (target) // double check ?
      return false;

if (move_is_castle(m)) // added
      return false; // note that in "pl_move_is_legal()" is true (bug ?)

  // Our move must be a blocking evasion or a capture of the checking piece
  target = squares_between(checksq, king_square(us)) | checkers();
  return bit_is_set(target, to) && (!pinned  || !bit_is_set(pinned, from)); // changed
}

And one such an amusing thing.
6.

Code: Select all

material.cpp - original code

template<Color Us> bool is_KBPsK(const Position& pos) {
    return   pos.non_pawn_material(Us)   == BishopValueMidgame
          && pos.piece_count(Us, BISHOP) == 1
          && pos.piece_count(Us, PAWN)   >= 1;
  }

Show must go on!

[mcostalba]

A simplification:

Code: Select all

template<bool FindPinned>
Bitboard Position::hidden_checkers(Color c) const {
  
  if (FindPinned && pinners)
      pinners &= ~st->checkersBB;
  

why not?

Code: Select all

Bitboard pinners = pieces_of_color(FindPinned ? opposite_color(c) : c) & ~st->checkersBB;
 

Short answer: because it is slower in !FindPinned case
Long answer: FindPinned is a template parameter (written with first char in uppercase) and it means that is evaluated at compile time, so when you see

Code: Select all

  
  if (FindPinned)

That instruction is _not_ evaluated at runtime, but the compiler already removes entirely the 'if' in case we are not in FindPinned.

Actually it removes also in FindPinned case, because in that case the statement after the 'if' is always executed and there isn't any branch instruction. Your code seems faster (becasue is more condensed), but actually you end up doing

Code: Select all

& ~st->checkersBB

also when not FindPinned.

Next, not for position.cpp.
4.
The status of half-open files matches to material events more than to pawn ones.
It seems to be advisable to move the next lines (and associated 3 functions) to mi-> .

No because is not the number of pawns, but their position on the board that is bounded to the half-open files. For instance I can have two pawns doubled or two pawns in chain, I have different file occupancy in those cases.