mjx-project / mjx

Mjx: A framework for Mahjong AI research
https://colab.research.google.com/drive/1m1wOT_K2YFtuV6IO7VgWk4ilVhTKqRFU?usp=sharing
MIT License
170 stars 19 forks source link

mjxprotoの変更したい箇所リスト #686

Closed habara-k closed 3 years ago

habara-k commented 3 years ago

概要

mjxprotoの変更したい箇所をまとめておいて、頃合いを見て一括変更する

変更希望箇所

理由: Eventを見るだけで、ただの流局なのか九種九牌なのかがわかるようにしたい

habara-k commented 3 years ago

等の表記があり、統一したい

sotetsuk commented 3 years ago

何がpublicで何がprivateなのかをより明確にしたいかもしれない。 ポーカーの論文では、public observationprivate observation のような単語も使って整理してたので、参考になるかもしれない。 (河は public observationで、closed handは private observation

sotetsuk commented 3 years ago

PettingZooに合わせると、Stateに次のプレイヤーが誰かの情報がほしいかもしれない。 ただこれは天鳳との兼ね合いを考えると微妙かもしれない。。。

sotetsuk commented 3 years ago

現在のハンドが入ってないが、いれるかどうか。。。

sotetsuk commented 3 years ago

TerminalもEventの中に入れたほうが自然か?

sotetsuk commented 3 years ago

PrivateInfo はミスリーディングか。(InfoがImperfect Information gameのInfoと同じように見える)

sotetsuk commented 3 years ago

見直して思うのは、どの情報が更新されて、どの情報がfixedなのかがわかりにくい。

sotetsuk commented 3 years ago

Draft => #694

方針

TODO

sotetsuk commented 3 years ago
habara-k commented 3 years ago

他にも、 No も加えた History が合ったほうがいいような気もするし

天鳳のlogにはNoがない => mjxproto形式で NoをHistoryには加えにくい

(教師あり学習では、例えばチーをできた状況で他家のポンが入ったとき、チーをせずNoを選んだ扱いにしているが、これは学習においてそこまで影響がないという仮定の元での妥協案である。厳密なプロトコルを作る今回の場合、ここを妥協しない方が良さそうな気がする)

habara-k commented 3 years ago

legal_actions も全てのdecision pointであれば便利

これはObservationUtils.legal_actions で達成可能という認識で良いでしょうか?

sotetsuk commented 3 years ago

他にも、 No も加えた History が合ったほうがいいような気もするし

天鳳のlogにはNoがない => mjxproto形式で NoをHistoryには加えにくい

(教師あり学習では、例えばチーをできた状況で他家のポンが入ったとき、チーをせずNoを選んだ扱いにしているが、これは学習においてそこまで影響がないという仮定の元での妥協案である。厳密なプロトコルを作る今回の場合、ここを妥協しない方が良さそうな気がする)

まあもし加えるとしたら、今の event_historyNo なしのまま据え置いて、 別に utils 以下に No もある完璧なhistoryも別で保持するという形になるかなあという感じですね。

sotetsuk commented 3 years ago

legal_actions も全てのdecision pointであれば便利

これはObservationUtils.legal_actions で達成可能という認識で良いでしょうか?

強化学習のときには

教師あり学習のときには、後者のパターンなので、同じくヒストリがないとダメだと思います。

habara-k commented 3 years ago

Observation.ObservationUtils 内にlegal_actionが存在するのはロジック的に大丈夫なのか?

Observationはあるplayerから観測したStateであるので、例えば他家が直前の捨牌をポンできるかどうかはわからない方が良い

legal_actionを付け加えるとするなら、State.legal_actionが良い気がする。 => 次が誰のターンか、という情報も合ったほうがいいような気もする も解決可能

habara-k commented 3 years ago

代替案

sotetsuk commented 3 years ago

代替案

  • legal_actionwhoごとに分けて、それぞれをState.PrivateObservation.legal_actionに格納する

僕の頭の中のイメージはこれでしたね。現状の実装とズレがありますね。ありがとうございます。 現状の legal_actionsprivate_observationprivate_observation.utils の下に移されるべきですね。

habara-k commented 3 years ago

legal_actions_historyを例えば

LegalActions
|- repeated Action actions

PrivateObservation
|- repeated LegalActions legal_actions_history

のようにもつ場合、private_observation.legal_actions_historypublic_observation.event_historyの対応関係をとるのが難しそう。

private_observation.draw_historypublic_observation.event_historyの内、who=自分, type=drawのものと順番に対応するので簡単に対応が取れるが、legal_actionsは自明な対応が取れなさそう)

habara-k commented 3 years ago

特徴量生成の高速化のため、repeated bool PublicObservation.under_riichi を加えてもいいかもしれない。

sotetsuk commented 3 years ago

PlayerIdx は本当に必要なのかも謎。結局、色んな場所で暗黙的にユーザをリストで持ち、 0 = 起家 を仮定している。( Score など) ただ、あったほうがわかりやすいのも事実ではある。

sotetsuk commented 3 years ago

先日のディスカッションメモ

sotetsuk commented 3 years ago

チェック項目

sotetsuk commented 3 years ago

あとの懸念はEventとActionをマージするか否かくらいか。これもやはり完璧な行動履歴を用意するかに依存している気がする。

sotetsuk commented 3 years ago

TakeAction しかメソッドを用意してないが、現状だと Observation は意思決定が必要な状況でしか生成されないので、終局時にその情報が渡されない。これはRLをする上で問題になるかもしれない。ダミーの行動を用意した方がよいか。

sotetsuk commented 3 years ago

player_idはシミュレータ側で設定しているが、いいのか? エージェント側がもっているべきとう説もある

sotetsuk commented 3 years ago

mjx.proto 変更による影響

更新手順

1. mjx.proto と必要な修正を更新

2. まず、python側が動くことを確認

もし mjconvert 以下で venv がなければ $ make venv してからアクティベートした方がよい。

$ cd mjconvert
$ make clean && make install
$ make pytest
$ make test-cli  // これはテストとしてちゃんと機能しているのか怪しい

現実には、 make clean && make installmake pytest を繰り返しながらコードを修正していく。 IDEやVSCodeでprotoの変更による AttributeError は検知してくれる場合が多い。 最後に make formats で整える。

3. C++側のテスト資材を作り直してコミット

2.でコマンドを実行したのと同じ、 mjconvert ディレクトリで

$ mjconvert ../tests/resources/mjlog ../tests/resources/json --to-mjxproto --verbose
$ git add ../tests/resources/json
$ git commit -m "update test resources"

4. C++側のテストを実行

CLionで変更とテストをしていく(あるいは makecmake-build-debug も頻繁に消す必要がありそう。

修正例

https://github.com/mjx-project/mjx/pull/702

sotetsuk commented 3 years ago

RoundTerminalState としては確かに utils の下に突っ込んでもいいけど、 Observation としては utils の上に置いておかないとダメかもしれない。

sotetsuk commented 3 years ago

RoundTerminal をどうするか?

  1. [そのまま] (スリムにして) utils の上へ動かす
  2. EventWinfrom_whoHand を加えて、 Tenpai のような EventType も追加して対処する
  3. [2.よりは現状との差分が少ないか] Event に新しく RoundTerminal typeを加えて utils の情報を terminal_info のようなattributeへ移す
    • 誰が、どの手で
  4. [天鳳] EventTypeとして AGARIE, RYUUKYOKU があり、それぞれ全員分の上がりや流局情報を保持
sotetsuk commented 3 years ago

protobufは最新版からoptionalが使える

optionalが使えるなら、例えば Action も本当は discardopenoptional が正しい

habara-k commented 3 years ago

裏ドラはHiddenStateUtils 以下に入る予定だが、立直した人が上がった場合、裏ドラの情報を何かしらの方法でpublicにするべきか?