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

Feature/fix first ac #969

Closed
wants to merge 10 commits into from
Closed

Conversation

fukatani
Copy link
Contributor

@fukatani fukatani commented Jun 9, 2021

まだ大分作りかけです。
大分できてきました。
Resolves #899

@kenkoooo kenkoooo marked this pull request as draft June 9, 2021 09:44
@fukatani fukatani force-pushed the feature/fix-first-ac branch 3 times, most recently from 34027ea to 82f692e Compare June 11, 2021 15:54
@kenkoooo
Copy link
Owner

もしレビュー可能な状態になりましたら Ready for review みたいなボタンを押していただいても良いですか?

@fukatani fukatani marked this pull request as ready for review June 13, 2021 01:19
@fukatani fukatani changed the title [Draft] Feature/fix first ac Feature/fix first ac Jun 13, 2021
@fukatani
Copy link
Contributor Author

@kenkoooo
コメントありがとうございます。
s3-clientを元に戻して、s3を叩いているところをatcoder_problems_backend/src/updaterというところに移しました。
まだまだツッコミどころあると思いますが、一度レビューしていただけませんか?

@fukatani
Copy link
Contributor Author

レビュー対応はできましたが、ローカルで走らせてみると、abc196_aのFirst ACがmasterでは21059853だったのが、21059856になってしまいます。
masterの方が正しいので、バグっています。調査中です。

Comment on lines 24 to 26
ids.push(cur.id.clone());
problem_ids.push(cur.problem_id.clone());
contest_ids.push(cur.contest_id.clone());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

たしか sqlx には Vec<&String> あるいは Vec<&str> を渡しても良かったはずです。

Copy link
Contributor Author

@fukatani fukatani Jun 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error[E0277]: the trait bound `Vec<&std::string::String>: Type<Postgres>` is not satisfied
  --> sql-client/src/first_ac_submissions.rs:47:19
   |
47 |             .bind(problem_ids)
   |                   ^^^^^^^^^^^ the trait `Type<Postgres>` is not implemented for `Vec<&std::string::String>`
   |
   = help: the following implementations were found:
             <Vec<&[u8]> as Type<Postgres>>
             <Vec<&str> as Type<Postgres>>
             <Vec<(T1, T2)> as Type<Postgres>>
             <Vec<(T1, T2, T3)> as Type<Postgres>>
           and 24 others

というコンパイルエラーが出て&Stringは入れられないようです。submission_idは整数なので、clone記述はやめました。

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

&strでもだめですか?

@fukatani
Copy link
Contributor Author

first_ac_submissionsのインターフェイスを修正しました。

レビュー対応はできましたが、ローカルで走らせてみると、abc196_aのFirst ACがmasterでは21059853だったのが、21059856になってしまいます。

この件はクロールがやりきれてなかったようで、クロールをやり直してbatch_updateするとmasterと同じ結果になりました。

@kenkoooo
Copy link
Owner

そもそもこの「コンテスト中の提出のうち非参加者によるものは全て除く」というアイディアは First AC だけでなく Sortest や Fastest にも適用できるのではないですか?

@fukatani
Copy link
Contributor Author

@kenkoooo

うーむ、ちゃんと仕様について相談すればよかったかもですね。

私が考えていたのは、shortestはコンテストに参加してなくても後でコードゴルフしてshortestにしたい人もいるため、shortestはコンテスト中もしくはコンテスト参加者に限らないのかなと思っていました。
fastestも同様にコンテスト後にも競われる性質が強いように感じたので、現行の仕様そのままにしていました。

どのsubmissionがx-estになるかの絞り方として、

  • コンテスト参加者によるコンテスト期間中のみx-estの権利がある
  • コンテスト参加者によるコンテスト開始以後全ての提出でx-estの権利がある
  • 任意の提出者によるコンテスト開始以後全ての提出でx-estの権利がある

という3つの考え方があるのですが、コンテスト中にほとんど決定されるfirstと違い、fastestとshortestで二番目の選択肢は違和感が大きいようにも思えます。
どうでしょう?

@fukatani
Copy link
Contributor Author

fukatani commented Jun 13, 2021

firstはコンテスト終了後(=解説公開後)はあまり意味が無い気がするので、本当はコンテスト期間中の提出に限った方がいいと思います。
その場合、コンテスト中誰からも解かれなかった問題はその後解かれても該当者なしとなります。

@kenkoooo
Copy link
Owner

x-est は次のように決めるものと考えています。

  • 次の提出集合の中からxが最も小さいもののうちidが一番小さいものをを選ぶ。
    • コンテスト中で、かつ、参加者による提出
    • コンテスト後の提出

ここで、First は x=id, Shortest は x=length, Fastest は x=execution_time と考えます。

コンテスト中に解かれなかったような問題では解説を見てもACするのが大変かと思いますし、First AC がコンテスト後の提出でも意味がいないとはあまり思いません。

@fukatani
Copy link
Contributor Author

完全に理解できました!
確かにkenkooooさんが提示していただいた仕様がよさそうですね。
幸い、修正は簡単そうなのでよかったです。

@kenkoooo
Copy link
Owner

ただ、提出集合を

  • A={コンテスト中で、かつ、参加者による提出}∪{コンテスト後の提出}
  • B={コンテスト中の提出}∪{コンテスト後の提出}

とした時に A ≠ B となるのは、writer/tester/admin がコンテスト中に提出し、かつ、それを公開設定にした場合のみなので、S3やLambdaやSQLにお金をかけてこのコーナーケースを取り除く必要があるかは未だによく分かっていません……

@fukatani
Copy link
Contributor Author

S3やLambdaやSQLにお金をかけてこのコーナーケースを取り除く必要があるかは未だによく分かっていません……

PRを作っておいてなんですが、これは私も微妙だなと思いました。
現象が少ないうちは、例えば提出一個一個をidで指定して弾くとかでもいいのかなと。
ユーザーからの不満や指摘がわんさか来たら自動で対応したいですけど。

ここはkenkooooさんの判断でペンディングしてもらって構わないのです。

私としては結構勉強になったんで、このPRに取り掛かれたこと自体はよかったです。
rustcのバグに気づけたりもしましたし。
rust-lang/rust#86265

@kenkoooo
Copy link
Owner

ありがとうございます。せっかく作り込んでいただいたところ大変申し訳ないのですが、一旦様子見でいきましょう…

@fukatani
Copy link
Contributor Author

いえいえ、色々とご助言ありがとうございました!
とりあえず、クローズします。必要になったらまたOpenしましょー

@fukatani fukatani closed this Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants