Conversation
| smallest_l, biggest_l = dfs(node.left) | ||
| smallest_r, biggest_r = dfs(node.right) |
There was a problem hiding this comment.
smallest, biggestという命名が理解しづらかったです。
今注目しているノードをnode、その左の子と右の子をleft, rightとすると、
left < node < rightでないといけない、というのは当然です。
ただ、leftをルートとする左側の部分木の中で、leftの値が一番大きいわけではないですよね。そうすると、biggest_lという命名の変数をnode.valの有効範囲の小さい方に使うのは疑問を抱きました。
There was a problem hiding this comment.
あと、末端から有効かどうかを検証していれば、left,node,rightの3つのノード間の関係のみで検証ができそうだなと思いました。
There was a problem hiding this comment.
ただ、leftをルートとする左側の部分木の中で、leftの値が一番大きいわけではないですよね。そうすると、biggest_lという命名の変数をnode.valの有効範囲の小さい方に使うのは疑問を抱きました。
min_of_left_subtree, max_of_left_subtreeみたいなのはいかがでしょうか
(追記: 文脈に沿ってないと思うので、このコメントは忘れてもらって大丈夫です。)
There was a problem hiding this comment.
末端から有効かどうかを検証する場合ですが
2
/
1
\
3
だとどうでしょうか?
There was a problem hiding this comment.
@Shunii85 nodeを根とする部分木がBSTである必要十分条件は何でしょうか?
There was a problem hiding this comment.
ただ、leftをルートとする左側の部分木の中で、leftの値が一番大きいわけではないですよね。そうすると、biggest_lという命名の変数をnode.valの有効範囲の小さい方に使うのは疑問を抱きました。
node.leftの部分木、node.rightの部分木が二分探索木であることが分かれば、あとはnode.valが有効範囲内にある事を確認すればいいのですがnode.leftの部分木での最大値をnode.valが下回ってしまうとあれ、なんでその最大値を持つノードは左に振り分けられてるの?右にいるべきじゃん!となってしまうのでnode.valはnode.leftの部分木の最大値を上回らなければなりません。node.rightの最小値がnode.valの上限となっているのも同じ理由です。
| @@ -0,0 +1,101 @@ | |||
|
|
|||
| ## step1: | |||
| ひとまずdfsで一番下の根からvalidateしていって再帰的にnode.right, node.leftに対してnodeがvalidかを判別していく。まずnode.right, node.leftがvalidである必要があるがそこはそれぞれをvalidateする際に早期にFalseをマークしておいて、すべてvalidateが終わった時にそのマークがFalseであるかで判断する。nodeがvalidであるかを判別するためにはnode.rightの最小値より小さく、node.leftの最大値より大きい必要がある。そこでdfs関数で判別したnodeのsmallest, biggestなnumberを返すようにする。葉に到達した際には必ずその根をvalidにする必要があるのでsmallestに無限大を、biggestにマイナス無限大を返すようにする。しかし、そうするとその根のnodeのleftのbiggestがマイナス無限大、rightのsmallestが無限大になってsmallest, biggestの更新が正しく行われずにvalidationが常にTrueになってしまう。そこでmin(node.val, smallest_l), max(node.val, biggest_r)を常に判断して返すようにしたが、これを葉付きのノード以外に毎回行うのは効率が悪い気がした。 | |||
There was a problem hiding this comment.
ひとまずdfsで一番下のノード(葉)からvalidateしていって、、、早期にTrueをマークしておいて、、、、
ですかね。
根と葉とそれ以外のノードを混同しているように感じました。
There was a problem hiding this comment.
下からvalidateしていって、どこかで引っ掛かったら早期にFalseをis_validにマークしてますね。できればFalseであることがわかった瞬間に早期returnしたいのですがdfs関数で進めてしまった為全部探索するまで終わらない不自然な感じになってしまいました。
| @@ -0,0 +1,101 @@ | |||
|
|
|||
| ## step1: | |||
| ひとまずdfsで一番下の根からvalidateしていって再帰的にnode.right, node.leftに対してnodeがvalidかを判別していく。まずnode.right, node.leftがvalidである必要があるがそこはそれぞれをvalidateする際に早期にFalseをマークしておいて、すべてvalidateが終わった時にそのマークがFalseであるかで判断する。nodeがvalidであるかを判別するためにはnode.rightの最小値より小さく、node.leftの最大値より大きい必要がある。そこでdfs関数で判別したnodeのsmallest, biggestなnumberを返すようにする。葉に到達した際には必ずその根をvalidにする必要があるのでsmallestに無限大を、biggestにマイナス無限大を返すようにする。しかし、そうするとその根のnodeのleftのbiggestがマイナス無限大、rightのsmallestが無限大になってsmallest, biggestの更新が正しく行われずにvalidationが常にTrueになってしまう。そこでmin(node.val, smallest_l), max(node.val, biggest_r)を常に判断して返すようにしたが、これを葉付きのノード以外に毎回行うのは効率が悪い気がした。 | |||
There was a problem hiding this comment.
これを葉付きのノード以外に毎回行うのは効率が悪い気がした。
(読み間違えていたらすみません。)葉以外のノードに対してもこのチェックは必要かと思います。
あるnodeがvalidであるには、nodeの左部分木の最小値よりも大きく、右部分木の最大値よりも小さいことが必要だと思います。 min(node.val, smallest_l), max(node.val, biggest_r)がないと、nodeの左の子よりも大きいことと右の子よりも小さいことのチェックになりそうです。
(文章が間違っていたので修正しました。5/6 23:43)
There was a problem hiding this comment.
smallest_l, biggest_l = dfs(node.left)
smallest_r, biggest_r = dfs(node.right)
ここのdfsでnode.right、node.leftのサブツリーはvalidであることが保証されてると想定して、
if not (biggest_l < node.val < smallest_r):
is_valid = False
ここで
nodeの左の子よりも大きいことと右の子よりも小さいことのチェック
これが済んでいると思います。
そして
return min(node.val, smallest_l), max(node.val, biggest_r)
ここをちゃんとleft, rightに葉をもつnodeと、部分木を持つnodeで処理を分けるとしたら
min_val = smallest_l if node.left else node.val
max_val = biggest_r if node.right else node.val
return min_val, max_val
こんな感じになると思います。
There was a problem hiding this comment.
min(node.val, smallest_l), max(node.val, biggest_r)を常に判断して返すようにしたが、これを葉付きのノード以外に毎回行うのは効率が悪い気がした。
これを、この処理は葉ノード以外は冗長である(なくてもいい)という主張だと思っていました。
return min(node.val, smallest_l), max(node.val, biggest_r)
より
min_val = smallest_l if node.left else node.val
max_val = biggest_r if node.right else node.val
return min_val, max_val
のほうが効率がいいのではという主張ですね。
もしそうだとすると、どれくらい速くなるのかについて考察があるとよいと思います。
There was a problem hiding this comment.
この処理は葉ノード以外は冗長である(なくてもいい)
個人的にこれが一番言いたかったことですかね。冗長さをなくした結果速度も上がるんじゃないかとついでに思ったのですが、Claudeに聞いてみると
return min(node.val, smallest_l), max(node.val, biggest_r)
これのmin, maxの関数オブジェクト呼び出しコストがかかるが、その差は無視できるものであると言われたので速度よりも可読性を意識した方が良さそうです。
There was a problem hiding this comment.
用語が正確でないように見えています。
ここをちゃんとleft, rightに葉をもつnodeと、部分木を持つnodeで処理を分けるとしたら
というコメントがあるのですが、葉=葉ノード(leaf = leaf node)とは、空でない子を持たないノードのことです。
https://ja.wikipedia.org/wiki/%E6%9C%A8%E6%A7%8B%E9%80%A0_(%E3%83%87%E3%83%BC%E3%82%BF%E6%A7%8B%E9%80%A0)
したがって、例えば「leftに葉を持つ」場合とは、node.leftが子を持たないノードになるので、
min_val = smallest_l if node.left else node.val
という条件でいうとnode.leftがTruthyになり、(空でない)部分木を持つ場合と一緒になってしまいます。
「葉付きのノード」に関する一般的な定義はないですが、leftとrightの少なくとも一方がleaf、ぐらいが一般的な解釈だと思うので、そうすると上記の説明のとおりになります。
また、これは文脈で判断はできますが、一般に単に部分木という場合は、空集合も部分木に含まれる場合があります(たとえば部分集合ということばには空集合も含まれることが多いです)。
問題を解けるかどうかは重要ですが、正しい言葉で会話ができるか?ということは、同等以上の強いシグナルになることが多いです。難しい問題では、厳密に解ききれるかどうかということ以上に、技術的に会話ができるかということが重要になるケースもあります。
また、それはそれとして、例えばbytecodeを生成させるなどして、差をある程度は定量的に把握することはできると思います。(普通の状況では概ね無視できるとは思いますが)
| smallest_r, biggest_r = dfs(node.right) | ||
| if not (biggest_l < node.val < smallest_r): | ||
| is_valid = False | ||
| return min(node.val, smallest_l), max(node.val, biggest_r) |
There was a problem hiding this comment.
結果的に正しく動くのですが、smallest/biggestという言葉をきちんと体現させるとしたら、smallest_rも含めるか、あるいは変数名を変えるか、厳密な体現は諦めてコメントするか、でしょうか。
私は変数名自体はわかりづらくはなかったですが、一瞬これで良いのかは気になったので、なぜこれでも正しく動くかのコメントはあった方が良いかもしれません(smallest_r<nome.valならすぐ上のifで捕まってvalidではない事になっているので、それゆえここではsmallest_rは考えなくてよい)
または、全体を早期リターンしていれば、嘘がなくなるとは思います(ただ、その場合に結果的に正しく動くことを読み手に考えさせる事になるかもしれません)
There was a problem hiding this comment.
ここでmin(node.val, smallest_l), max(node.val, biggest_r)とやっているのはnode.leftやnode.rightが葉だった場合にそれらのsmallestやbiggestを無限大に飛ばしているのでそのエッジケースを解消するために書いてます。もしnode.left, node.rightが葉でなく部分木である場合は全く無駄な処理となります。
min_val = smallest_l if node.left else node.val
max_val = biggest_r if node.right else node.val
return min_val, max_val
こう書いた方がより説明的でエッジケースに適切に対処したコードになりますね。
全体を早期リターンした方がいいのは全くもってその通りでございます。ただ、一番下の根から順番にvalidateしていく方針だとdfs関数で深掘る以外思い浮かばなかったので、step2での上の根から順番にvalidateしていく方を今後採用すると思います。
There was a problem hiding this comment.
sasanquaneufさんのコメントは、BSTではなかった場合に、smallest/biggestというのは嘘になるよねという話だと思います。
ここでmin(node.val, smallest_l), max(node.val, biggest_r)とやっているのはnode.leftやnode.rightが葉だった場合にそれらのsmallestやbiggestを無限大に飛ばしているのでそのエッジケースを解消するために書いてます。
ここで、「葉」はどういった意味で使っていますか?コードを参照すると、空ノード(None)の意味で使っているように見えますが、葉は子供がいないノードのことです。
There was a problem hiding this comment.
私の解釈が間違っておりました...
Noneを葉と同等で扱っていました。正確にはnode.leftとnode.rightが空ノードだった場合に無限大に飛ばしているですね。
There was a problem hiding this comment.
もとのコメントが言葉足らずですみません、
BSTではなかった場合に、smallest/biggestというのは嘘になるよね
が意図で、
min_val = min(node.val, smallest_l, smallest_r)
などとしないと一般のBSTでない場合においては本当のsmallestではない、というのが言いたいことでした。
ただし、本当のsmallestを返却しなくても、is_validのチェックに引っかかるのでここからはsmallest_rを省いてよいことになります。
その意味で全体の動作の問題はないのですが、ただ変数名が厳密な意味と異なることになり、読み手としては「このコードは本当に正しいのか?」という疑いを持ちながら検証しないといけないので、負荷が発生します。
以下、具体的にある読み手のシミュレーションをしてみましょう。
変数名は、読み手がメンタルモデルを作るときのヒントになるので、読み手はまず
「smallest_lという名前だから、smallestが入っているのだな」
というメンタルモデルを構築しようとすると思うのですが、このsmallest_lの値が何かとコードを読むと
return min(node.val, smallest_l), max(node.val, biggest_r)
となっているので、左側はこのサブツリーのsmallestではなく、あれ?と思ってコードを精読します。
精読する(例えばいくつかのケースで動作をシミュレーションしてみる)と、このsmallestにはBSTの場合にはsmallestが正しく入るが、BSTではない場合でsmallestが入らないケースがあり、構築したメンタルモデルと違っているな?ということになります。
最終的に、すぐ上の分岐との関連性を見出して、is_validが保たれている場合においてはsmallestであることが保証されるが、is_validではない場合にはsmallestではない値を格納しているのだな、という理解に至ります。
このとき、たとえば変数名やminを取る操作のところにコメントがあったりすると、
「smallest_lという名前だから、smallestが入っているのだな」
というメンタルモデルが出来上がる前に、「BSTではない場合はsmallestという仮定・意図が崩れるのだな」ということをメンタルモデルに組み込めるので、読み手の人自身が「一度誤解した内容を正しい内容にする」という工程を省略できます。
これが、読み手に優しい、読みやすいという意味の一つだと思います。
There was a problem hiding this comment.
または、全体を早期リターンしていれば、嘘がなくなるとは思います
についての補足になると思いますが、各dfs()の呼び出しの後に、if not is_valid: return 0, 0みたいにearly returnしておくと良いですかね。ゴミが返ってくるとわかるので。
is_valid = Falseの後も同様にreturn 0, 0しておくと良さそうです。
There was a problem hiding this comment.
Claudeに提案させてみるとこのようになりました。
def isValidBST(root):
def dfs(node):
# returns (is_valid, min_val, max_val)
if not node:
return True, float('inf'), float('-inf')
left_valid, left_min, left_max = dfs(node.left)
right_valid, right_min, right_max = dfs(node.right)
if not left_valid or not right_valid:
return False, 0, 0
if not (left_max < node.val < right_min):
return False, 0, 0
min_val = min(node.val, left_min)
max_val = max(node.val, right_max)
return True, min_val, max_val
valid, _, _ = dfs(root)
return valid
その地点でのノードでinvalidということが分かればFalse, 0, 0となってこれ以降の判定が全て無効になるということが分かりやすいですね。
| @@ -0,0 +1,101 @@ | |||
|
|
|||
| ## step1: | |||
| ひとまずdfsで一番下の根からvalidateしていって再帰的にnode.right, node.leftに対してnodeがvalidかを判別していく。まずnode.right, node.leftがvalidである必要があるがそこはそれぞれをvalidateする際に早期にFalseをマークしておいて、すべてvalidateが終わった時にそのマークがFalseであるかで判断する。nodeがvalidであるかを判別するためにはnode.rightの最小値より小さく、node.leftの最大値より大きい必要がある。そこでdfs関数で判別したnodeのsmallest, biggestなnumberを返すようにする。葉に到達した際には必ずその根をvalidにする必要があるのでsmallestに無限大を、biggestにマイナス無限大を返すようにする。しかし、そうするとその根のnodeのleftのbiggestがマイナス無限大、rightのsmallestが無限大になってsmallest, biggestの更新が正しく行われずにvalidationが常にTrueになってしまう。そこでmin(node.val, smallest_l), max(node.val, biggest_r)を常に判断して返すようにしたが、これを葉付きのノード以外に毎回行うのは効率が悪い気がした。 | |||
There was a problem hiding this comment.
木を"正しい向き"、根が下になるように書くと一番下の根というのは正しいですが、leetcodeとかの図は逆ですね。あと、根は常に端になりますね。意図は通じますが一応。
There was a problem hiding this comment.
(atmaxstarさんの補足があったので一応)ここでは一番下のノード(葉ノード)を根とする部分木のことを指していますね。
| smallest_l, biggest_l = dfs(node.left) | ||
| smallest_r, biggest_r = dfs(node.right) |
There was a problem hiding this comment.
末端から有効かどうかを検証する場合ですが
2
/
1
\
3
だとどうでしょうか?
| @@ -0,0 +1,101 @@ | |||
|
|
|||
There was a problem hiding this comment.
典型コメント集の「用語を雑に使わない」に関連していると思いますが、全体的にあまり通じないか不正確な文章になっていると思います。
There was a problem hiding this comment.
今見返してみるとstep1の説明でかなり目的語を省いているのでわかりづらい文章になってました...
ざっと翻訳してみると、あるnodeを根とする木が二分探索木であることを検証するにはnode.leftを根とする部分木とnode.rightを根とする部分木がそれぞれ二分探索木であり、かつ
node.leftが根となる部分木の最大値 < node.val < node.rightが根となる部分木の最小値
である事を検証する必要がある。そこで、nodeに対しての左右の部分木が二分探索木であることの検証とそれぞれの木全体での最小値、最大値の返却を再帰的に行うことで元のrootが二分探索木である事を検証できる。
といった感じでしょうか。
There was a problem hiding this comment.
目的語の省略ではなく、単純に用語の使い方が不適切なことについてのコメントでした。省略されているのは、文脈で補えるかもしれませんが、標準的な用語の使い方から外れると、エスパーしないと意図が読み取れないかなと。例えばですが、
一番下の根
葉に到達した際には必ずその根をvalidにする必要がある
といった表現から、「根」を標準的な意味である「一番上のノード」の意味で使っていないと推測しました。
新しい方は正しいと思います。
There was a problem hiding this comment.
「一番下の根」ではなく「一番下のノード」や、そのノードを「根」とする部分木みたいにきちんと定義に則って使うべきですね。
読み手にとって理解しやすい言葉の取り扱いに気をつけるようにします。
| # when in node A, if A.val is greater than biggest val of A.left tree and smaller than smallest val of A.right tree, A is binary tree | ||
| # ↑ in this case, A.right and A.left must be valid binary trees as well, so I'm gonna validate both in advance using dfs and return False early |
There was a problem hiding this comment.
s/binary tree/binary search tree/ でしょうか?
ここではどれが必要条件・必要十分条件・十分条件に相当していますか?
There was a problem hiding this comment.
コメントに脳死でbinary treeと書いてしまいました...
binary treeだと単に各ノードが最大2つの子を持つ木で、binary search treeが左の子 < 親 < 右の子が全ての部分木で成り立つbinary treeですね。BST→Binary Treeは成立しますが、Binary Tree→BSTは成立しませんね。
There was a problem hiding this comment.
必要条件とかの話はBSTとbinary treeについてではなく、英文の方についてですね。一文目で十分条件を提示しているように読めますが、二文目に追加で必要条件もありますと書いているように見えます。そして、両方チェックしますとありますね。論理構造としてかなり不自然で、BSTの定義を理解していないか、必要条件・十分条件・必要十分条件を区別していないと解釈されうる文章であるように思います。
when in node A, if A.val is greater than biggest val of A.left tree and smaller than smallest val of A.right tree, A is binary tree
↑ in this case, A.right and A.left must be valid binary trees as well, so I'm gonna validate both in advance using dfs and return False early
There was a problem hiding this comment.
英語のコメントは実際に対面で説明するならどんな感じ説明しようかのシミュレーションで書いていて、1行目に思考法、2行目に実装方法を説明するみたいな感じで書きました。ただそうなると、
A.right and A.left must be valid binary trees
この説明を1行目に移したほうが自然ですね。1行目で特定のノードを根とする2分木が2分探索木であるための条件を書き、2行目でdfsを使った再帰的な方法で解きますといった感じで説明する様に。
There was a problem hiding this comment.
説明方法の問題というよりも、「ノードAの値が、左部分木の最大値より大きく、右部分木の最小値よりも小さい時、ノードAを根とする木はBSTである」というのは、単純に誤りですかね。BSTの必要条件の一つに過ぎないので。
when in node A, if A.val is greater than biggest val of A.left tree and smaller than smallest val of A.right tree, A is binary tree
There was a problem hiding this comment.
左の子 < 親 < 右の子が全ての部分木で成り立つ
左の木の最大値 < 親 < 右の木の最小値
でしょうかね。
There was a problem hiding this comment.
説明方法の問題というよりも、「ノードAの値が、左部分木の最大値より大きく、右部分木の最小値よりも小さい時、ノードAを根とする木はBSTである」というのは、単純に誤りですかね。BSTの必要条件の一つに過ぎないので。
when in node A, if A.val is greater than biggest val of A.left tree and smaller than smallest val of A.right tree, A is binary tree
この説明は仰る通り誤りですね。ただその説明にwhen A.right and A.left are valid binary treesと左と右の部分木がvalidな2分探索木であるという前提条件を加えれば十分条件になると思います。ChatGPTに簡単に英訳させたんですけどこんな感じにすれば説明からの実装方法のつながりが分かりやすくなると個人的には思います。
A tree rooted at node A is a valid Binary Search Tree if:
both A.left and A.right are valid Binary Search Trees, and
the maximum value in the subtree rooted at A.left < A.val < the minimum value in the subtree rooted at A.right.
Implementation approach:
Use Depth-First Search recursively to verify whether each subtree is a valid Binary Search Tree while also returning the minimum and maximum values of each subtree. Using this information, determine whether the tree rooted at the current node is a valid Binary Search Tree.
There was a problem hiding this comment.
必要十分条件になっているかということですかね。
十分条件だけど必要条件ではないものだとBSTである木を誤ってBSTでないと判定してしまう可能性がありますね。
- A.left, A.rightを根とした木も二分探索木である
- A.leftを根とした部分木の最大値 < A.val < A.rightを根とした部分木の最小値
この2条件はBSTの必要十分条件になっていると思います。十分条件(2条件 → BST)は再帰的に各部分木のBST性質が保証されるので成立し、必要条件(BST → 2条件)もBSTの定義から直接導けるので成立します。
面接でどこまで厳密性を証明すればいいかはまだよくわかってないですが...
| # the left must be less than parent.val and bigger than smallest val of ancestor | ||
| # the right must be bigger than parent.val and smaller than biggest val of ancestor |
There was a problem hiding this comment.
ancestorはparentより上の祖先の想定でしたが、
the left must be less than parent.val and bigger than smallest val of ancestor
8
/
4
/
3
上記の場合で4をparentとしてみたときの3に対して誤りでした。正しく書くなら
the left must be less than parent.val and bigger than the lower inherited from ancestor
the right must be bigger than parent.val and less than the upper inherited from ancestor
みたいな感じですかね。
There was a problem hiding this comment.
ancestorはparentより上の祖先の想定
こちらも標準的な用語の使い方から外れますね。
問題:(https://leetcode.com/problems/validate-binary-search-tree/description/)
言語: Python3