Che 19 game endings #48

bennyrubanov · 2023-11-14T04:29:16Z

not sure how to implement without using Chess class
running into typeErrors when trying to use chess class from chess.ts
added in some modifications to chess.ts to try to reintroduce isThreefoldRepetition() (which was stripped out previously)

bennyrubanov · 2023-11-14T04:30:54Z

src/metrics/misc.ts

+        flags: parseInt(move.flags, 10)
+      });
+
+      if (chess.isThreefoldRepetition()) {


realizing the if statements below should prob be outside of the loop? not sure actually

EllAchE · 2023-11-14T08:15:19Z

src/metrics/misc.ts

+    if (gameEnd === '[Termination "Normal"]') {
+      if (lastMove.originalString.includes('#') || this.chess.isCheckmate()) {
+        this.gameEndings['checkmate'] = (this.gameEndings['checkmate'] || 0) + 1;
+      } else if (result === '[Result "1/2-1/2"]') {
+        this.gameEndings['draw'] = (this.gameEndings['draw'] || 0) + 1;
+      } else if (this.chess.isStalemate()) {
+        this.gameEndings['stalemate'] = (this.gameEndings['stalemate'] || 0) + 1;
+      } else if (this.chess.isInsufficientMaterial()) {
+        this.gameEndings['insufficient material'] = (this.gameEndings['insufficient material'] || 0) + 1;
+      } else {
+        this.gameEndings['resignation'] = (this.gameEndings['resignation'] || 0) + 1;
+      }
+    } else if (gameEnd === '[Termination "Time forfeit"]') {
+      this.gameEndings['time out'] = (this.gameEndings['time out'] || 0) + 1;
+    } 


thihs seems wrong, stalemate and draw are not mutually exclusive

yeah u right stalemate and insufficient material should be nested under draw

EllAchE · 2023-11-14T08:16:07Z

src/metrics/misc.ts

+    let fiftyMovesCount = 0;
+    let boardPositions:  { [board: string]: number } =  {};
+    let threefoldRepetitionFound = false; // check if threeFoldRepetition has been found
+    const lastBoardString = JSON.stringify(game[game.length - 1].board) + this.chess.turn();


this doesn't capture castling state, or EP

yes look at comments line 276. im starting to think this might be the reason for why multiple threefold reps are being identified, but im not 100% sure and it seems unlikely

EllAchE · 2023-11-17T06:48:01Z

cjsmin/src/chess.ts

+    this._positionCounts = new Proxy({} as Record<string, number>, {
+      get: (target, position: string) =>
+        position === 'length'
+          ? Object.keys(target).length // length for unit testing
+          : target?.[this._trimFen(position)] || 0,
+      set: (target, position: string, count: number) => {
+        const trimmedFen = this._trimFen(position)
+        if (count === 0) delete target[trimmedFen]
+        else target[trimmedFen] = count
+        return true
+      },
+    })
+  }
+
+  private _trimFen(fen: string): string {
+    // remove last two fields in FEN string as they're not needed when checking for repetition
+    return fen.split(' ').slice(0, 4).join(' ')


can we take all this stuff out? We aren't using it right?

Yes. Removed all positionCounts and related changes from latest: #52

EllAchE · 2023-11-17T06:49:02Z

src/metrics/misc.ts

+          blackWins: result === '[Result "0-1"]' ? 1 : (result === '[Result "1/2-1/2"]' ? 0.5 : 0),
+          whiteWins: result === '[Result "1-0"]' ? 1 : (result === '[Result "1/2-1/2"]' ? 0.5 : 0),
+          ties: result === '[Result "1/2-1/2"]' ? 1 : 0,


a switch statement might be more readable

sure, added in #52

EllAchE · 2023-11-17T06:49:51Z

src/metrics/misc.ts

+        } else if (this.chess.isInsufficientMaterial()) {
+          this.gameEndings['insufficient material'] = (this.gameEndings['insufficient material'] || 0) + 1;
+        } 
+      } else {


might note that this is an implicit assumption that metadata is valid

what do you mean? this is buried under multiple if statements that prob ensure metadata validity

EllAchE · 2023-11-17T06:50:09Z

src/metrics/misc.ts

+    const lastMove = game[game.length - 1].move;
+
+    if (gameEnd === '[Termination "Normal"]') {
+      if (lastMove.originalString.includes('#') || this.chess.isCheckmate()) {


is the or needed?

EllAchE · 2023-11-17T06:50:43Z

src/metrics/misc.ts

+
+    if (gameEnd === '[Termination "Normal"]') {
+      if (lastMove.originalString.includes('#') || this.chess.isCheckmate()) {
+        this.gameEndings['checkmate'] = (this.gameEndings['checkmate'] || 0) + 1;


maybe you could just instantiate game endings instead of the boolean check

not sure i follow

EllAchE · 2023-11-17T06:51:30Z

src/metrics/misc.ts

+        this.gameEndings['threefold repetition'] = 
+          (this.gameEndings['threefold repetition'] || 0) + 1;
+        threefoldRepetitionFound = true;
+      }


this is probably good enough, 99.999% of games that meet this criteria & it was last move are probably 3FR

EllAchE · 2023-11-17T06:51:52Z

tests/metrics.test.ts

+      metadataMetric.processGame(Array.from(cjsmin.historyGenerator(game50MovesRule[0].moves)),
+      ['m', 'et']);
+
+      expect(metadataMetric.gameEndings['fifty-move rule']).toEqual(1);


3FR & stalemate tests would be nice too

i had implicit test through console logging of analysis on 3 game test set and feel pretty confident these are working properly, but feel free to add though if you think impt

EllAchE

Approving this but I'd like to see most of the changes i suggested bc will be more readable

bennyrubanov · 2023-11-18T11:38:49Z

i have added all changes suggested to most recent branch, #52, then will merge that bad boy in and resolve merge conflicts

* track game castling, piecelevelmoveinfo logs fix * queen/king side castling count, more log fixes * attempting to identify game endings using chess class * changing how chess is pushed into metadatametric * three-fold repetition attempt, everything else working * removed some logs * addressing logan's comments, logic fixes, still need ep and castling * identify 3-fold repetition and test for 50-move rule * kd ratio by piece values added * remove assists from maps * making sure all most metrics account for ties. some changes to distance metric * fixing tests * excluding getMoveDistanceSingleGame test for now * partial decompressor shell script * adding empty aggreagte/logging methods for index.ts for loop to run * attempts at getting partial decompressor working * running analysis after every data chunk, but analysis logs not printing to console * log fixes and trying to remove undefined typerror on GameChunks * running partial decompresser yields multiple files that are analyzed one at a time, but sometimes files are deleted before analysis script is run * suggestions from PR #48 * changes from comments on #50 * fileReader changes to capture games with unknown lines, adding analysis queues * making sure missing data for ratings is addressed * misc fixes/changes * moves and distance metrics fixes/completion, adding all metric outputs to aggregate method return * killstreak function typeerror fixes * output to results.json working, updates to maxkillstreak * small updates * remove mentions of assist from kdmetric, final_aggregate_analysis script drafting * more aggregate analysis (captures) * killstreaks, mate and assists, promotions * distance metrics * lockfile to compression funct, final aggregate analysis finished metrics, small changes elsewhere * fixing metric.test.ts errors * fixed max kill streaks checker, split index files based on use * bug fixes * fixed unit tests * updating gitignore to not include results.json * next * not great implementation of queues * reverted back to pre-queue work, added in queue only to index_with_decompressor file * attempts at preventing overwriting * extra changes * tiny changes * test * resolving merge conflicts when merging shell-script-and-chunk-analysis into final-analysis-script * not great implementation of queues * attempts at preventing overwriting * extra changes * finished merge conflict fixes from merging shell script in final analysis * remove .VSCodeCounter folder from repository * logan's review changes

bennyrubanov added 4 commits November 13, 2023 11:33

track game castling, piecelevelmoveinfo logs fix

da64b2e

queen/king side castling count, more log fixes

080c1ea

Merge remote-tracking branch 'origin/main' into kdRatioFunct

3584e3f

attempting to identify game endings using chess class

eb6bc3c

bennyrubanov commented Nov 14, 2023

View reviewed changes

bennyrubanov requested a review from EllAchE November 14, 2023 04:31

bennyrubanov added 3 commits November 14, 2023 13:56

changing how chess is pushed into metadatametric

d77baf0

three-fold repetition attempt, everything else working

3d0ef1b

removed some logs

7fbaa73

EllAchE reviewed Nov 14, 2023

View reviewed changes

bennyrubanov added 2 commits November 14, 2023 16:26

addressing logan's comments, logic fixes, still need ep and castling

a4d166b

identify 3-fold repetition and test for 50-move rule

133996f

bennyrubanov marked this pull request as ready for review November 16, 2023 06:25

bennyrubanov requested a review from EllAchE November 16, 2023 06:25

bennyrubanov mentioned this pull request Nov 16, 2023

calculate kd ratios by piece values #50

Closed

EllAchE reviewed Nov 17, 2023

View reviewed changes

EllAchE approved these changes Nov 17, 2023

View reviewed changes

bennyrubanov closed this Nov 18, 2023

bennyrubanov added a commit that referenced this pull request Nov 18, 2023

suggestions from PR #48

868fc0c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Che 19 game endings #48

Che 19 game endings #48

bennyrubanov commented Nov 14, 2023

bennyrubanov Nov 14, 2023

EllAchE Nov 14, 2023

bennyrubanov Nov 14, 2023

EllAchE Nov 14, 2023

bennyrubanov Nov 14, 2023

EllAchE Nov 17, 2023

bennyrubanov Nov 18, 2023

EllAchE Nov 17, 2023

bennyrubanov Nov 18, 2023 •

edited

Loading

EllAchE Nov 17, 2023

bennyrubanov Nov 18, 2023

EllAchE Nov 17, 2023

bennyrubanov Nov 18, 2023

EllAchE Nov 17, 2023

bennyrubanov Nov 18, 2023

EllAchE Nov 17, 2023

EllAchE Nov 17, 2023

bennyrubanov Nov 18, 2023

EllAchE left a comment

bennyrubanov commented Nov 18, 2023

Che 19 game endings #48

Che 19 game endings #48

Conversation

bennyrubanov commented Nov 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bennyrubanov Nov 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EllAchE left a comment

Choose a reason for hiding this comment

bennyrubanov commented Nov 18, 2023

bennyrubanov Nov 18, 2023 •

edited

Loading