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 clarification for piece representation in infer_pieces.py #23

Conversation

nayan2167
Copy link
Contributor

  • Modified __PREDS_DICT in infer_pieces.py to use piece names as keys and piece Ids as values (e.g., {'B': 0, 'K': 1, 'N': 2, ...}).
  • Updated the __max_piece() function in infer_pieces.py to access tops using the piece names instead of numeric indices (e.g., tops[0][0]["B"]).
  • Updated the is_empty_square(square_probs) function in infer_pieces.py to utilize the modified __PREDS_DICT values.

These changes were made to improve the clarity and accuracy of piece representation within the infer_pieces.py script. The __PREDS_DICT dictionary now associates piece names with their respective Ids, allowing for more intuitive referencing. The __max_piece() function and is_empty_square() function has been adjusted accordingly to accommodate the updated piece representation, ensuring correct functionality.

@nayan2167 nayan2167 marked this pull request as ready for review July 11, 2023 21:39
lc2fen/infer_pieces.py Outdated Show resolved Hide resolved
lc2fen/infer_pieces.py Outdated Show resolved Hide resolved
@nayan2167
Copy link
Contributor Author

Hi @davidmallasen please review the changes I made, I mistakenly created a new PR(#26) for this commit pardon me for that, I am closing it. also #25 is the same as this issue I would like to fix it, one more question should I create a fresh PR for that or can I commit to this branch?

@davidmallasen
Copy link
Owner

I added the review comments. No problem on opening and closing the other PR :)
Issue #25 now sould be related to this now as #24 was merged. If you would like to fix it, please go ahead and add those commits to this PR. I will add the trigger to fix issue #25 also when this PR is merged.

@davidmallasen davidmallasen linked an issue Jul 17, 2023 that may be closed by this pull request
@nayan2167
Copy link
Contributor Author

nayan2167 commented Jul 18, 2023

Hi @davidmallasen
__max_piece, __determine_promoted_piece, __determine_most_probable_white_piece, __determine_most_probable_black_piece, infer_chess_pieces these are the functions where I found the magic numbers and updated them with values corresponding to __PREDS_DICT if needs further changes let me know.

Here is the sample output

Promoted piece (__determine_promoted_piece): Q
----------------------------------------
Most probable White piece (__determine_most_probable_white_piece) : P
----------------------------------------
Most probable Blacj piece (__determine_most_probable_black_piece): n
----------------------------------------
Warning: the selected model is not accurate enough to predict a balanced board configuration
        Please consider providing the previous FEN, selecting a different model, or performing
        transfer learning on that model
Infer chess piece (infer_chess_pieces): ['N', 'q', 'R', 'p', '_', 'B', 'r', 'r', 'n', 'p', 'P', 'Q', 'b', 'R', 'N', 'N', 'r', 'N', 'p', 'q', 'P', 'P', 'P', 'B', 'n', 'n', 'k', 'P', '_', '_', 'P', 'B', 'n', 'r', 'n', 'N', 'P', 'p', 'Q', 'R', 'r', 'b', 'B', 'B', 'q', 'q', 'p', 'q', 'Q', 'r', 'p', 'B', 'n', 'p', 'p', 'b', 'K', 'B', 'b', 'N', 'q', 'p', 'R', '_']

@davidmallasen
Copy link
Owner

@nayan2167 Could you also add this for the kings (numbers 1 and 8) in

white_king = max(pieces_probs_sort, key=lambda prob: prob[0][1])

and
pieces_probs_sort, key=lambda prob: prob[0][8], reverse=True

?

@nayan2167
Copy link
Contributor Author

@nayan2167 Could you also add this for the kings (numbers 1 and 8) in

white_king = max(pieces_probs_sort, key=lambda prob: prob[0][1])

and

pieces_probs_sort, key=lambda prob: prob[0][8], reverse=True

?

Done, also magic numbers removed from __sort_pieces_list

@davidmallasen
Copy link
Owner

Great, thanks @nayan2167 !

@davidmallasen davidmallasen merged commit 23f1f6d into davidmallasen:master Jul 18, 2023
@nayan2167 nayan2167 deleted the inverse-_PREDS_DICT-in-infer_pieces.py branch July 18, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove magic numbers in infer_pieces.py Clarify piece representation in infer_pieces.py
3 participants