-
Notifications
You must be signed in to change notification settings - Fork 0
139. Word Break.md #42
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
base: main
Are you sure you want to change the base?
Conversation
| ## Step3 | ||
|
|
||
| ```python | ||
| import re |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
消し忘れでしょうか。
| - 結局どの長さのwordのendかを、全部持たないといけない? | ||
| - じゃあ結局線形ではできない? | ||
| - アホコラ自体の実装は合ってるはず? | ||
| - TrieTreeクラスもAhoCoraクラスもコンストラクタで色々な操作をやっているが、あまり良くないのだろうか??(よくわからない) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C++ だと、コンストラクタでの Exception の扱いが難しいので、Constructor は軽くしておいて、init() のような関数を別に用意するというようなテクニックがあります。
が、Python はあんまり気にしないでいいんじゃないでしょうか。(Python 詳しい人の意見は別にあるかもしれませんが。)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
なるほど参考になりました、ありがとうございます
| result = re.match(pattern, s) | ||
| if not result: | ||
| return False | ||
| return True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return re.match(pattern, s) is not Noneで良さそうですね
| self.depth = depth | ||
|
|
||
| class TrieTree: | ||
| def __init__(self, words: str) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
あんまり自信ないんですが、words: Iterable[str] とかですかね...少なくともstrではないですね
| while True: | ||
| if index >= len(s): | ||
| return False | ||
| node = trie.move_to_child_node(node, s[index]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trieに対してmove_to_child_nodeという名前のメソッドを呼ぶと、レシーバであるtrieの状態が変わることを期待する気がします(trieにchild nodeへmoveしろと言っているように見えるので)。
この実装のようにnodeを呼び出し側で取り回す方針なら、trieじゃなくてTrieTreeNodeのメソッドとして、(Noneを返すかもしれないので)find_child_nodeみたいな名前の方がいいかもしれないです
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
なるほど、こういう感覚はなかったので参考になりました、ありがとうございます
メソッドでインスタンスの状態が変わるかどうかというのが、結構重要なんですね
| self.char_to_child_node = {} | ||
| self.is_end_of_word = False | ||
|
|
||
| class TrieTree: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
あんまりTrieTreeという名前は見ないかもしれません。Trieでいいかなという気がします
(日本語のトライ木というのも個人的には違和感あってトライでいいと思いますが、あまり自信はないです)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Treeを入れたい場合は、PrefixTreeとかですかね。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ありがとうございます、知らなかったです
| trie = TrieTree(wordDict) | ||
|
|
||
| @cache | ||
| def can_split_removing_first_this_length(removed_length): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
関数名、引数の名前がわかりにくい気がしました。splitできるかを判定している対象がsなのも良く見ないとわからないと思います。
プライベート関数にして、
def _can_split_from(self, s: str, trie: TrieTree, start: int) -> bool:
...とかにする方がやってること表してるかなという気がしました
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
そうですね、引数多くなるのが微妙かと思ってこうしましたが、結果的にわかりにくくなっちゃいましたね
|
|
||
| - Trie木のなかに、sのprefixでwordDictにあるものを全て取ってくる機能をつけてるが、いるのかな | ||
| - 今がwordDictのなかにある単語の終わりかどうかはすぐわかるので、それで十分なような? | ||
| - treeをwordDictから具体的にbuildするのは、クラスの初期化コードに含まない方がいいのだろうか |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
含めてもいいんじゃないかなと思います。基本的にはTrieの初期化処理の責任をどこまでと見るかということですよね
| can_segmented_so_far[0] = True | ||
| for current_index in range(1, len(s) + 1): | ||
| for previous_index in reversed(range(current_index)): | ||
| if s[previous_index: current_index] in wordDict and can_segmented_so_far[previous_index]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s[previous_index:current_index]
| for last_segmented in reversed(range(length)): | ||
| if not _can_be_segmented_finishing_this_length(last_segmented): | ||
| continue | ||
| if s[last_segmented: length] in wordDict: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if s[last_segmented:length] in wordDict:
| self.char_to_child_node = {} | ||
| self.is_end_of_word = False | ||
|
|
||
| class TrieTree: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Treeを入れたい場合は、PrefixTreeとかですかね。
| for last_segmented in reversed(range(length)): | ||
| if not _can_be_segmented_finishing_this_length(last_segmented): | ||
| continue | ||
| if s[last_segmented: length] in wordDict: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wordDictはsetにしておいた方が良さそうですかね。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
勝手にsetだと勘違いしてました、ありがとうございます
| https://github.com/philip82148/leetcode-arai60/pull/8/files | ||
|
|
||
| - Aho-Corasickでの実装(間違い) | ||
| - Solutionクラスで、can_split[i - node.depth + 1]のcheckだけにすると、前にたどれるだけたどる以外も最適の場合があるのでだめ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
これ、いい指摘で、["a" * 11, "a" * 13] で作れない文字列を表現しようと思うと、Trie 上を移動しているだけでは状態の数が足りないはずですね。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
この目的では微妙に合わない感じがします。
下には Aho-Corasick が使える問題が少し挙げてあります。
https://cp-algorithms.com/string/aho_corasick.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ありがとうございます、みてみます
| - じゃあ結局線形ではできない? | ||
| - アホコラ自体の実装は合ってるはず? | ||
| - TrieTreeクラスもAhoCoraクラスもコンストラクタで色々な操作をやっているが、あまり良くないのだろうか??(よくわからない) | ||
| - chatGPTにこの悩みを相談したら、[@classmethod](https://docs.python.org/3/library/functions.html#classmethod)とかもいいですよと言ってきた。これどうなんだろう |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ある種の factory を用意するという意図でしょうか。Python ではそこまで見ない気もしますが、他の言語では時々見るかもしれません。
| def move_to_next_node(self, node: TrieTreeNode, next_char: str) -> TrieTreeNode: | ||
| while next_char not in node.char_to_child_node: | ||
| node = node.failure_link | ||
| if not node: | ||
| return self.trie.root | ||
| return node.char_to_child_node[next_char] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
認知されてるかもしれませんが、バグを見つけたのでご報告です。
テストケース
s = "dogs"
wordDict = ["dog","s","gs"]で通らないと思うので、確認してみて下さい。
この実装で上記のテストケースを走らせると、"dog"まで見て次の"s"を探索する際、whileループに入って、node.failure_link(="gs"の"g")に移動すると思います。次にwhileの判定文でnode.char_to_child_nodeを確認すると、"s"が見つかるので"gs"の"s"に該当するノードが返ってくると思うのですが、これは想定された挙動でない気がします(ここで探索が終了してしまう)
node.failure_linkで移動した後にnode.is_end_of_wordを確認して、Falseの場合はrootに戻す処理にしないといけない気がします。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
コメントありがとうございます。
すみません、自分の理解がおかしいかもしれないのですが、「"gs"の"s"に該当するノード」で探索が終了するのが想定されていないというのはなぜでしょうか?
「"gs"の"s"に該当するノード」に、"gs"の終わりであることと"s"の終わりであることの両方の情報が含まれていれば、wordDict内のすべての文字列を見逃すことはないと思いました。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
すいません、私の理解が間違ってるかもなんですが、私の想定している動き方が、
- dogが見つかる
- can_split更新(can_split = [True, False, False, True, False])
- 最長サフィックスを探すが見つからない(ogもgもwordDictには存在しない)
- sが見つかる
- can_split更新(can_split = [True, False, False, True, True])
- 全部舐めたので探索完了
なんですが、この実装だと、
- dogが見つかる
- can_split更新(can_split = [True, False, False, True, False])
- dogのgの子にsがないのでgsのgへ移動(L232~L233)
- gの子sがnext_charなのでループを抜ける(L232)
- gsのsが返ってくる(L245)
- can_split更新(can_split = [True, False, False, True, False])
- 全部舐めたので探索完了
となってると思います。
gsのsはnode.depth=2なので、この実装だと最後の要素がTrueにならないように見えます。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
あ、AhoCoraskアルゴリズム自体ではなく、この問題に関していえば通らないということですね。L160に書いてあるように、貪欲にたどって言ってもダメというバグですね(わかりにくい場所にコメントがあってすみません)
この問題に関していえば結局末尾のwordをぜんたんさくしないといけないので、この箇所にあるodaさんのコメントにあるように、AhoCoraskを使うこと自体が向いてない気がします。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
まだ認識ズレがあるような気がします。
今回のテストケースは貪欲に辿ってもうまくいくと思います(dogs = dog + s)
例えば雑ですが以下のようなコードで対応できると思います。
class Solution:
def wordBreak(self, s: str, wordDict: List[str]) -> bool:
can_split = [False] * (len(s) + 1)
can_split[0] = True
trie_with_failure = AhoCorasick(wordDict)
node = trie_with_failure.trie.root
for i, c in enumerate(s):
if c in node.char_to_child_node:
node = node.char_to_child_node[c]
if not node.is_end_of_word:
continue
can_split[i + 1] = can_split[i - node.depth + 1]
# check suffix & update can_split
node = node.failure_link
while node:
if node.is_end_of_word:
can_split[i + 1] = can_split[i - node.depth + 1]
node = node.failure_link
node = trie_with_failure.trie.root
return can_split[-1]一方で、上記の解法でも貪欲にやっていくとうまくいかない問題は解消していません。
例えばs = "aaaaaaa", wordDict = ["aaaa","aaa"]のケースでは失敗します。
このコードでは"aaa"を2回見つけて、最後の"a"がwordDictにないので、到達不可能と判定しますが、aaaaから見つければ上手くいきます。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
すみません、絡んでおいて私もよくわからなくなってきました。
私のAho-Coraskの理解は、単語を見つけたらそのサフィックスも(大きい順に)順次見つけていくアルゴリズムというもので、今回の実装がそういう動きになっていないのに違和感があったのかもしれません。
ただ貪欲の考え方の違いだというのがイマイチ腹落ちしておらず。。
ちなみに貪欲の定義の違いだとすると、私のコードが通らずオリジナルのコードが通るようなテストケースって何か挙げられますでしょうか。。(私の頭の整理に付き合って頂く形になっちゃってすいません、、)
尚、2.についてはyes、3.は(上手く工夫すれば出来るのかどうか)良くわかりません。。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- は少し工夫するとできるんですかね。
Aho-Corask はサフィックスを大きい順に見つけていくわけなので、can_split のフラグと組み合わせると O(文字列長 * 辞書サイズ) ですか。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TORUS0818 送ってくださったコードよく読まずに返信してしまったので変な返信してしまったかもしれないです🙏
Trie木のnodeについて
自分の→suffixの最長情報を常に保持しているのが不変条件
torusさんの→自分のと基本一緒だが、単語を見つけ次第情報を捨てる
can_split配列をTrueにする条件について
自分の→suffixの最長情報のみ調べて、その手前で切れるかだけ調べる
torusさんの→nodeの保持しているis_end_of_word状態をfailureを辿って全て調べる(ただしnodeが失敗せず通常通り進んで、かつend_of_wordでない場合は調べないので、今のnodeではend_of_wordではないが他の単語で実はend_of_wordの場合もれてしまう)
で合ってますかね…?
s = "abc"で、wordDict = ["ab", "abc"]の場合、おそらく自分の場合だけ通りますね。
Trie木のnodeの状態については自分の、can_split配列をTrueにする条件についてはtorusさんのを調べることで、正しいコードになるような気がしてきました(?)ただ、この場合いちいちたどることにはなるので、辞書サイズが計算量にかけられて大変ですね。
@oda
辞書サイズがかけられていても「線形」にはなるんですかね(言葉の定義が曖昧ですみません)。「文字列の長さ」に関しては線形ですね。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
これ、can_split条件のtorusさんの、(ただし〜
以降は、そもそもそんな状態はなかったですね(is_endは伝播してるので)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
オートマトンの話の線形は通常入力長に対して線形という議論をします。
計算量理論の興味とエンジニアとしての興味はちょっとずれているので、計算量ではなくて計算時間について考えるといいと思います。
philip82148
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(今すでにコメントされている部分以外は)いいと思います!
URL: https://leetcode.com/problems/word-break/description/
Given a string s and a dictionary of strings wordDict, return true if s can be segmented into a space-separated sequence of one or more dictionary words.
Note that the same word in the dictionary may be reused multiple times in the segmentation.