Skip to content

Commit

Permalink
Simplification of partial_insertion_sort formula.
Browse files Browse the repository at this point in the history 
Passed STC:
https://tests.stockfishchess.org/tests/view/6590110879aa8af82b9562e9
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 134880 W: 34468 L: 34355 D: 66057
Ptnml(0-2): 476, 16060, 34220, 16243, 441

Passed LTC:
https://tests.stockfishchess.org/tests/view/659156ca79aa8af82b957f07
LLR: 2.94 (-2.94,2.94) <-1.75,0.25>
Total: 60780 W: 15179 L: 14996 D: 30605
Ptnml(0-2): 27, 6847, 16464, 7020, 32

closes #4955

Bench: 1338331
  • Loading branch information
@FauziAkram @Disservin
FauziAkram authored and Disservin committed Jan 4, 2024
1 parent 444f03e commit 5546bc0
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/movepick.cpp
View file
Expand Up @@ -300,7 +300,7 @@ Move MovePicker::next_move(bool skipQuiets) {
endMoves = generate<QUIETS>(pos, cur);

score<QUIETS>();
partial_insertion_sort(cur, endMoves, -1960 - 3130 * depth);
partial_insertion_sort(cur, endMoves, -3330 * depth);
}

++stage;
Expand Down

9 comments on commit 5546bc0

@PedanticHacker
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FauziAkram and @Disservin, the result of -1960 - 3130 is -3130 and not -3330. Please fix this.

@dav1312
Copy link
Contributor

@dav1312 dav1312 commented on 5546bc0 Jan 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the result of -1960 - 3130 is -3130

explain

@Disservin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PedanticHacker There's nothing to fix here. The patch just removed the subtraction from the equation?
See the corresponding test on fishtest https://tests.stockfishchess.org/tests/view/659156ca79aa8af82b957f07,
which shows that -1960 - 3130 * depth can simply be replaced by -3330 * depth without being a regression.

@PedanticHacker
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My calculator is an asshole. Please excuse the nag.

@PedanticHacker
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The correct result is -5090.

@Disservin
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, but it still doesnt matter.

@peregrineshahin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The correct result is -5090.

  1. https://en.cppreference.com/w/cpp/language/operator_precedence
  2. Simplification patches are not exclusive to non functional changes

@PedanticHacker
Copy link
Contributor

@PedanticHacker PedanticHacker commented on 5546bc0 Jan 4, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally missed the operator precedence.

It would be nice if calculations such as -1960 - 3130 * depth include the parenthesis, to be explicit which operation has precedence, just to avoid confusion. I think that seeing a calculation written as -1960 - (3130 * depth) is more explicit.

It now doesn't make sense for -3330 * depth, but for other calculations such as -1960 - 3130 * depth was.

@vdbergh
Copy link
Contributor

@vdbergh vdbergh commented on 5546bc0 Jan 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PedanticHacker Operator precedence is primary school stuff....

Please sign in to comment.