mjx-project / mjx_mjai_translater

Translation server between mjx and mjai protocols
MIT License
2 stars 0 forks source link

observationの差分を取得する関数のテストを追加する #32

Closed nissymori closed 3 years ago

nissymori commented 3 years ago

とりあえず一局分追加する。

sotetsuk commented 3 years ago

Protocol bufferのクラスが encode_jsondecode_json のようなメソッドを持っているらしい

nissymori commented 3 years ago

差分を取得する関数のテストケースを追加しました。

sotetsuk commented 3 years ago

お疲れ様です!ありがとうございますー。 絶対パスがコードにごっそり入ってるのにテストが通ってるのが気持ち悪いので、本当にCI上でコードが動いてるか確認してみてくださいー。 @nissymori

nissymori commented 3 years ago

grpc関係のfileが読み込めないerrorがでます。 pathはこれまで走っていたテストと同じ依存関係なのでうまくいくと思います。 make protos的なものをgithub actionに追加してgrpc用のfileをtest時に作る必要があるってことでしょうか? @sotetsuk

sotetsuk commented 3 years ago

そうですね。CIにも追加が必要だと思います!

nissymori commented 3 years ago

追加しておきます! Run testsの下に追加する感じで大丈夫ですかね?

sotetsuk commented 3 years ago

いや、わりと最初の方に入れた方がいいと思います!

nissymori commented 3 years ago

it submodule init git submodule update —init でmjxをsubmoduleにできるのでしたっけ? .gitmoduleにはmjxのurlがあるんですけど、ciでerrorがでます。 @sotetsuk

sotetsuk commented 3 years ago

了解です!僕がやっておきますー 👍 ありがとうございます!!

sotetsuk commented 3 years ago

@nissymori これで git submoduleproto ファイルの生成までは問題ないはずですー 👍

sotetsuk commented 3 years ago

image

nissymori commented 3 years ago

ありがとうございます! まだ何箇所かpathに問題があるみたいなのでfixします!

nissymori commented 3 years ago

メモ 今make protosで作られたmjx_services_pb.rbrequire "mjx_pb"が動かなくて詰まっている。 ローカルでは$LOAD_PATH.unshift(__dir__) unless $LOAD_PATH.include?(__dir__)requireする前に付けることで対応している。 ci上で$LOAD_PATHを追加する必要がある。

sotetsuk commented 3 years ago

多分なんですけど、 bundle install みたいなのでインストールしてからテストする想定だと思うので、相対パスでインポートしているのが妙な気がします。

sotetsuk commented 3 years ago

ローカルでも一回インストールしてから使えるか試してみるのがいいかもしれないです

nissymori commented 3 years ago

テストする前にmjx_pbのあるdirectoryを$LOAD_PATHに追加することでrequireの問題は解決しました!

nissymori commented 3 years ago

テスト適切に実行できています。

nissymori commented 3 years ago

ツモぎり以外の情報もobservaitonのfileに含まれていますが、ツモぎりと関係ないところは無視でよろしいですかね

sotetsuk commented 3 years ago

いや、これくらいはツモ切りも無視しないで実装した方が効率いいと思います

sotetsuk commented 3 years ago

ツモ切りのエージェント動かすのが最初の目標ですけど、なるべく普通に実装していった方がいいので

nissymori commented 3 years ago

一旦、ツモぎりに関係ある部分(局の最初、局の途中でのツモぎり) はテスト終わりました。 @sotetsuk

ツモ切りのエージェント動かすのが最初の目標ですけど、なるべく普通に実装していった方がいいので target_playerのチーやポンについてもテストを書くって感じですかね?

nissymori commented 3 years ago

@sotetsuk extract_differenceではevent_historyの差分だけを撮とるようにして、 covert_event()の引数をevent_historyの差分, observation, target_playerにするのが良いかなと思いました。 疑問点はTODOとして下のコードに書いています。 extract_difference のところのみテストを通しています。

 def extract_difference(previous_history = @_mjx_event_history, observation)
 # event_historyの単純な差分をreturn
end

def convert_to_mjai_action(observation, history_difference, target_player)
     #@drawの追加
     #if then でmjai_actionに変換 例 typeがdiscardなら[draw, discard]  typeがチーなら[chi] それぞれ中身はmjai_action
     return mjai_actions
end

def observe(observation, target_player) -> mjai_actions
   history_difference = extract_difference(previous_history = @mjx_history, observetion)
   mjai_actions = convert_to_mjai_actions(observation, history_difference, target_player)
   return mjai_actions
end

def do_action(mjai_action)  #簡易版
     responses = (0...4).map() do |i|
        @players[i].respond_to_action(action_in_view(action, i, true))  
  #action_in_viewを実装する(playerによって?にかえたるする部分) iがplayerを識別するidに対応している。
  #respond_to_actionの部分がclientと通信する関数
     end
     return responses
end

mjai_new_actions = observe(observation, target_player)
mj_new_actions.each do |action|
  responces = do_action(action)
end
sotetsuk commented 3 years ago

target_playerの情報からhistory_differenceのうち、target_playerが行ったeventを取り出す

これはおかしくないですか?全部のobservationを全てのプレイヤーに送らないといけないですよね?

sotetsuk commented 3 years ago

あと、mjconvertのときも言いましたけど、基本的に変換する関数とかは、要素一つに対して作用させるようにしてください。

sotetsuk commented 3 years ago

また後でもう少し見て確認します🙇‍♂️

nissymori commented 3 years ago

target_playerの情報からhistory_differenceのうち、target_playerが行ったeventを取り出す

これはおかしくないですか?全部のobservationを全てのプレイヤーに送らないといけないですよね?

直前のobservationから次に順番が回ってくるまでに起こったeventがplayer毎にobservationとして逐次的にTransServerに渡されていくという認識であっていますでしょうか?

observatioinの渡さ方がまだ完璧にイメージできていない気がします。

nissymori commented 3 years ago

また後でもう少し見て確認します🙇‍♂️

ありがとうございます!

sotetsuk commented 3 years ago

直前のobservationから次に順番が回ってくるまでに起こったeventがplayer毎にobservationとして逐次的にTransServerに渡されていくという認識であっていますでしょうか?

mjxはstate lessなので、エージェントが必要なときに必要な情報を全て渡します。 mjaiの動作が逐次的なので、それに合わせて変換する必要があります。

例えば、プレイヤーA, B, C, Dがいて、A, Bをmjxのプレイヤー、C, Dをmjaiのプレイヤーを使うとすると、

  1. mjxのシミュレータからAにObservationが渡され、Aが結果を返す(ObservationはそれまでのEventHistoryを全て含む)
  2. mjxのシミュレータからBにObservationが渡され、Bが結果を返す
  3. mjxのシミュレータがTransServerにObservationを渡し、do_actionをしてCから結果が返ってくる(このとき、TransServerに最後に来たObservationとの差分を計算する)
  4. mjxのシミュレータがTransServerにObservationを渡し、do_actionをしてDから結果が返ってくる(差分はCのツモ?とCのDiscard、Dのツモ)
  5. mjxのシミュレータからAにObservationが渡され、Aが結果を返す(ObservationはそれまでのEventHistoryを全て含む)
  6. mjxのシミュレータからAにObservationが渡され、Bが結果を返す(ObservationはそれまでのEventHistoryを全て含む)
  7. mjxのシミュレータがTransServerにObservationを渡し、do_actionをしてCから結果が返ってくる(差分はAのツモ?AのDiscard、Bのツモ?BのDiscard、Cのツモ)

みたいになります

sotetsuk commented 3 years ago

extract_difference のテストと挙動はこれで大丈夫だと思います!他に追加する点なければマージしちゃってください 👍

nissymori commented 3 years ago

了解です!mergeします! convertの設計はしっかり検討します。! observationの説明ありがとうございました!