Conversation
| * 取り出すたびに, 2個のペアをpushしている. 全体で1個の増加 | ||
| * heapの中は多くてもk個の要素 | ||
| * O(klog(k)) | ||
|
|
There was a problem hiding this comment.
(reviewerになっていませんが、立ち寄ったのでコメントします)
アプローチですが、作業手順が丁寧に書かれていてとても分かりやすいと感じました。
計算量の根拠もあり、とても読みやすいです。
| heapq.heappush(candidates_heap, (nums1[smallest_idx1] + nums2[smallest_idx2 + 1], smallest_idx1, smallest_idx2 + 1)) | ||
| already_in_heap.add((smallest_idx1, smallest_idx2 + 1)) | ||
| return result | ||
| ``` |
There was a problem hiding this comment.
上のアプローチがそのまま反映されていて読みやすいと感じました。
こちらのコードを参考にコーディングしてみたのですが、
idx1, idx2などの変数を結構打ち間違えることがありました(nums1, nums2もですが)。
打ち間違いをなくすために変数に数値をいれないほうがいいかもしれないと思いました(自分がコーディングをあまりしたことがないので起こっていることなのかもしれません)。
There was a problem hiding this comment.
nums1 nums2のように今回は数字入っている状態で引数に渡されていたので, 対応関係をとるためにidx1 idx2のように数字を入れていましたね.
数字使わない方法だと fst_nums fst_idxとかsec_nums sec_idxとかですかね. タイプ量増えるので打ち間違えとのトレードオフではありますが.
| next_idx_of_idx1 = cur_smallest_idx1 + 1 | ||
| next_idx_of_idx2 = cur_smallest_idx2 + 1 | ||
| if next_idx_of_idx1 < len(nums1) and (next_idx_of_idx1, cur_smallest_idx2) not in seen_idx1_idx2_pairs: | ||
| heapq.heappush(sum_idx1_idx2_tuple_heap, (nums1[next_idx_of_idx1] + nums2[cur_smallest_idx2], next_idx_of_idx1, cur_smallest_idx2)) |
There was a problem hiding this comment.
流石に変数名が長く、一行を読むのに苦労する印象がありました。
もう少しシンプルな変数名にして、説明的になるならば変数にコメント・型ヒントを残すと良いかなと思いました。
There was a problem hiding this comment.
これ見て, 命名は割としつこくてもいいのかなと思ったのですが, 何回も登場する変数と呼び出し回数が少ない関数では話が違そうですね.
next_idx1とかnext_idx2とかの方が簡潔ですね.
There was a problem hiding this comment.
擬態か迷彩に見えるみたいなことを言ってたことがありますね。
https://docs.google.com/document/d/11HV35ADPo9QxJOpJQ24FcZvtvioli770WWdZZDaLOfg/edit?tab=t.0#heading=h.fcs3httrll4l
| ```python | ||
| from typing import List | ||
| import heapq | ||
|
|
There was a problem hiding this comment.
class の周りには2つ空行を開けることが一般的かと思いますが、チームのスタイルガイドによると思います。
https://peps.python.org/pep-0008/#blank-lines
| if idx2 + 1 < len(nums2) and (idx1, idx2 + 1) not in already_in_heap_or_taken: | ||
| already_in_heap_or_taken.add((idx1, idx2 + 1)) | ||
| heapq.heappush(candidate, (nums1[idx1] + nums2[idx2 + 1], idx1, idx2 + 1)) | ||
| return result No newline at end of file |
There was a problem hiding this comment.
GitHub上で赤丸が表示されているものと思いますが、基本的にはエディタ内でInsert Final Newlineのような設定を有効にすることをオススメしています。
具体的な議論はいくつかあると思いますが、こちらが参考になるかもしれません。https://stackoverflow.com/q/729692/16193058
| `log(2 + diff)` | ||
|
|
||
| つまり, `M`と`N`がほぼ同じ値を取るときは, Setの使用の有無は関係ないが, | ||
| `M`と`N`の差が激しい時はSetを使用しない方法の方が効率的になる. |
There was a problem hiding this comment.
すみません、詳しく議論を追うには時間が足りなかったのですが、Setを使用する方法の方がheapに重複を入れずに済むから効率的になる、という理解であってますか?
重複が発生しないような状況ならSetを使用するオーバーヘッドだけ無駄に処理がかかる、ということですかね 🤔
There was a problem hiding this comment.
ここでは, Setを使用する方法 = 余分な候補をheapに入れる方法としています.
反対に, Setを使用しない方法 = 各idxごとにとった回数を保存して, 余分な候補をheapに入れない方法としています.
余分な候補を入れる状況は, 例えば, (1, 0)をとっていないのに(2, 0)をheapに入れるなどの状況ですね.
今回考えたのは, 余分な候補を入れることによるオーバーヘッドです.
結論として, 余分な候補を入れる(Setを使う方法の)オーバーヘッドはMとNの差が激しい時以外は無視できそうと考えました.
There was a problem hiding this comment.
あ、なるほど、Setでダブりを排するのではなくて、taken_countを用いる方法がSetを使用しない方法なのですね。勘違いしていました、ありがとうございます!
| _, smallest_idx1, smallest_idx2 = heapq.heappop(candidates_heap) | ||
| result.append((nums1[smallest_idx1], nums2[smallest_idx2])) | ||
| num_taken += 1 | ||
| if smallest_idx1 < len(nums1) - 1 and (smallest_idx1 + 1, smallest_idx2) not in already_in_heap: |
There was a problem hiding this comment.
自分は smallest_idx1 + 1 < len(nums1) と書くことが多いのですが、趣味の範囲だと思います。
https://leetcode.com/problems/find-k-pairs-with-smallest-sums/description/