Skip to content

Conversation

@vondele
Copy link
Member

@vondele vondele commented Apr 29, 2020

This reopens #1563 for the same reasons:

Integrate the tuning branch in master

Tuning parameters is commonly performed in fishtest, based on an infrastructure present in a separate 'tune' branch. However, the current situation has the following disadvantages:

  • 'tune' is out-of-sync with master
  • The tuning workflow is unclear to new devs, requires more merges/commits than necessary.
  • Diffs of tuning tests in fishtest are unreadable, leading to tests being approved without checking. This is a security risk, and leads to silly mistakes (e.g. approving a tuning test, which doesn't contain a TUNE()).
  • The tuning branch contains unrelated changes, and is not really maintained.

Merging the branch in master fixes these issues.

There is no performance overhead of including the tuning branch:

Passed STC:
LLR: 2.96 (-2.94,2.94) [-3.00,1.00]
Total: 109710 W: 21990 L: 22032 D: 65688
http://tests.stockfishchess.org/tests/view/5add9b8c0ebc5975a378488a

Binary size is nearly unchanged (<0.5% increase).

This commit is essentially unchanged from the existing code, with small edits:

remove unused stuff
create tune.cpp instead of appending it somewhere
fix license dates
fix name (encoding) typo

The comment by @snicolet that this is specialized C++ code still holds (#1563 (comment)), but I don't think this should stop us from integrating in-tree. The code has proven its value over the years.

No functional change.

The purpose of the code is to allow developers to easily and flexibly
setup SF for a tuning session. Mainly you have just to remove 'const'
qualifiers from the variables you want to tune and flag them for
tuning, so if you have:

int myKing = 10;
Score myBonus = S(5, 15);
Value myValue[][2] = { { V(100), V(20) }, { V(7), V(78) } };

and at the end of the update you may want to call
a post update function:

void my_post_update();

If instead of default Option's min-max values,
you prefer your custom ones, returned by:

std::pair<int, int> my_range(int value)

Or you jus want to set the range directly, you can
simply add below:

TUNE(SetRange(my_range), myKing, SetRange(-200, 200), myBonus, myValue, my_post_update);

And all the magic happens :-)

At startup all the parameters are printed in a
format suitable to be copy-pasted in fishtest.

In case the post update function is slow and you have many
parameters to tune, you can add:

UPDATE_ON_LAST();

And the values update, including post update function call, will
be done only once, after the engine receives the last UCI option.
The last option is the one defined and created as the last one, so
this assumes that the GUI sends the options in the same order in
which have been defined.

No functional change.
@FauziAkram
Copy link
Contributor

I would really love this to get merged!

@vondele vondele closed this in c527c3a May 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants