Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve connection games #768

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

RainRat
Copy link
Contributor

@RainRat RainRat commented Mar 27, 2024

  1. Change evaluate.cpp to also take into account connectPieceTypes when evaluating how close to connectN
  2. Change evaluate.cpp to also take into account connectValue. There's not many misere variants to go on, but reversing the sign is a start.
  3. Expand symphony and picaria to not inherit from tictactoe. There was an empty connectPieceTypes when inheriting from tictactoe.

@ianfab
Copy link
Member

ianfab commented Mar 27, 2024

Thanks. Have you done any playing strength testing on the eval changes? https://github.com/ianfab/variantfishtest is a simple option if you need a tool to run tests.

@RainRat
Copy link
Contributor Author

RainRat commented Apr 17, 2024

  1. Taking into account connectPieceTypes when evaluating how close to connectN: Is necessary for a basic understanding of the position. The change would not affect variants that don't use connectPieceTypes. Currently the only variant that uses it is Three Musketeers, which is not a connectN game, but a collinearN game. The change is more to prepare for future games like Align4, which may be worth testing once it is developed.
  2. Changing evaluate.cpp to also take into account connectValue.
  • Games with connectValue=win would be unaffected.
  • connectValue=loss is currently used by two games: Three Musketeers, which would be unaffected because it uses collinearN instead of connectN. That leaves Connect 4 Misere. The suggestion to reserve the sign on evaluation is indeed based on intuition, since one does not want the engine to work toward a loss condition. It's almost just a placeholder for future work, since misere variants may require totally different scaling. I was considering waiting for more misere variants but I didn't want to forget about this. I know AntiLine4 was also mentioned, but don't know if there are any others worth considering.
  • connectValue=draw have not considered, since nothing uses it yet. I suppose that it should skip the connectN evaluation entirely.
  1. This is more a bug fix since connectPieceTypes wasn't working correctly when inheriting.

So to proceed, I could look into

  • Trying to figure out how to scale connectValue + connectN based on just Connect 4 Misere
  • Check for connectValue=draw and skip the connectN evaluation.
  • Trying to root cause why connectPieceTypes isn't working correctly when inheriting.

@ianfab
Copy link
Member

ianfab commented Apr 17, 2024

Thanks, if basically no existing variants are affected by the eval changes regression testing is not required. Regarding the inheritance problem, I think this is because you are inheriting from variants with different (connection) piece types, so one needs to override connectPieceTypes in the inherited variants.

For reference, the below code should be what narrows it down to IMMOBILE_PIECE when defining tic-tac-toe, and then further narrows down piece_set(IMMOBILE_PIECE) & CUSTOM_PIECE_1 to an empty set.

connectPieceTypes = connectPieceTypes & pieceTypes;

@RainRat
Copy link
Contributor Author

RainRat commented May 4, 2024

It seems the solution is to leave connectPieceTypes unchanged but have a new connectPieceTypesTrimmed in use during the game.

@ianfab
Copy link
Member

ianfab commented Jul 8, 2024

Thanks, the fixes for evaluation, passing adjudication, and connection piece type inheritance look good to me. However, the optimizations of the connect adjudication are less clear cut to me. As I would like to be able to evaluate the bugfixes and optimizations independently, may I ask you to separate the optimizations into a different PR?

@ianfab
Copy link
Member

ianfab commented Jul 8, 2024

To link the corresponding items, this PR fixes #800 and #801.

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.

2 participants