MobilityArea

[Rocky640]

Include squares occupied by mobile pawns in the MobilityArea

Bench # 7600729

Passed STC
LLR: 2.95 (-2.94,2.94) [-1.50,4.50]
Total: 8157 W: 1644 L: 1516 D: 4997

And LTC
LLR: 2.97 (-2.94,2.94) [0.00,6.00]
Total: 26086 W: 4274 L: 4051 D: 17761

[joergoster]

Well done!

[Rocky640]

Thank you guys.
It is great to be challenged by your observations and ideas in the forum and your stimulating comments.

[mcostalba]

Congratulations! A real SPRT[0,6] patch after a vert loooong time.

I would prefer this version to the other one becuase it is easier to simplify.

For instance, once is committed I would remove the attacks, becuase are already take care in other parts of evaluation and I would just write:

Bitboard freePawns[] = { (shift_bb<DELTA_S>(pos.pieces()) & pos.pieces(WHITE, PAWN)) ^ pos.pieces(WHITE, PAWN),
                         (shift_bb<DELTA_N>(pos.pieces()) & pos.pieces(BLACK, PAWN)) ^ pos.pieces(BLACK, PAWN) };

// Do not include in mobility squares protected by enemy pawns or occupied by our blocked pawns or king
Bitboard mobilityArea[] = { ~(ei.attackedBy[BLACK][PAWN] | freePawns[WHITE] | pos.pieces(WHITE, KING)) ,
                            ~(ei.attackedBy[WHITE][PAWN] | freePawns[BLACK] | pos.pieces(BLACK, KING)) };

[mcostalba]

Ooops, I have misread the code, the correct one is this:

Bitboard blockedPawns[] = { (shift_bb<DELTA_S>(pos.pieces()) & pos.pieces(WHITE, PAWN)),
                            (shift_bb<DELTA_N>(pos.pieces()) & pos.pieces(BLACK, PAWN)) };

// Do not include in mobility squares protected by enemy pawns or occupied by our blocked pawns or king
Bitboard mobilityArea[] = { ~(ei.attackedBy[BLACK][PAWN] | blockedPawns[WHITE] | pos.pieces(WHITE, KING)) ,
                            ~(ei.attackedBy[WHITE][PAWN] | blockedPawns[BLACK] | pos.pieces(BLACK, KING)) };

[tomtor]

Congratulations !!!

[lucasart]

I prefer Marco's version, although it's not functionally equivalent. I think it should be tested before to commit this patch, because it's cleaner and easier to simplify.

[mstembera]

@lucasart
Why test Marco's version before this patch? Should it not be tested as a simplification against the fast version of this patch?

[Rocky640]

This 383 patch is "one simple idea", but is made of 3 elements
a) pawn push
b) ....and only from rank 4 and beyond
c) pawn captures

Marco's version is really nice indeed and covers a) perfectly (but not b or c)

Whatever plan we choose, it is clear that we will need to run more tests, either simplification tests, or numgame tests.

We can either
PLAN 1) Forget about this 383 and 384 patch, and test (a) alone (Marco's) against current master, then test if (a+b) is better than (a), then test if adding (c) is improvement (3 tests)
OR
PLAN 2) pull this already tested 383, try to remove (c), then try to remove (b) (2 tests)
(the pull request 384 can wait)
OR
PLAN 3) run a 25M numgames match directly @ltc between Marco's version and (unpulled) request 384, than pull the winner, than try to add/remove just (b), than try to add/remove just (c) (3 tests)
OR
some other plan (let me know)

PLAN 2 seems natural, and will take only 2 more tests (well up to 4 since we have to run STC and LTC)
PLAN 1 make sense if we believe in simplicity first, but might need 1 more test,

Either way is fine with me.

[lucasart]

@Rocky640: I don't think c) is useful. It's the job of the qsearch to resolve captures, and you should write evel assuming that the position is quiet. If a pawn can capture (legally), there's a good reason why it can't. That being said, we have evaluate_threats() which violates this principle, because the horizon effect is still pervasive when you're in qsearch or close to it. But for orthogonality's sake, I'd rather have this only in one place.

@mstembera: you're probably right. it's easier to just commit and test simplification after.

[Rocky640]

Summary of the day:

1. A simplification test was run by Lucas, Marco's version against "unofficial master 383", and this failed STC
   So a) b) c) is better than a) alone

2. A simplification test was run by me to remove c), and passed STC and LTC
   Your intuition about remove captures was right !

So no more c) and less noise from now on !
And our new "unofficial master" is a) + b) 

3. A parameter tweak [0,4] test is now ready to start for
   "a) and only from rank 3 and beyond" against "a) + b)"

 If it pass, (I seriously doubt) 
 the new "unoffical master" would be  "a) and only from rank 3 and beyond"
 (Lucas discussed he might try some tweaking on the pawn PSQT rank_2)

 If not, another parameter tweak might be tried (again, no great expectations here...)
"a) and only from rank 5 and beyond"  against      "a) + b)"

4. in any case, I will open a new pull request with the winner, using Marco's style.

[lucasart]

@Rocky640: agree. a+b written using "Marco's style" seems like the right patch to commit.

Whether or not the rank2/3 condition can be removed and compensated by PSQT is another test/patch to be tried after IMO. For now, it appears that the rank2/3 condition is useful.

[Rocky640]

Agree. We did enough verifications.

Thank you Lucas, and Marco for suggestions and comments which helped improve and simplify the patch.
I think we covered the main points and can proceed with #385 for the conclusion.

Also, a special thanks to all people who contribute workers.