Updates related to discarded rows default #89
Conversation
|
Related to filecoin-project/rust-fil-proofs#1219 |
|
This an alternative, but based on #88 |
cryptonemo
force-pushed
the
update-discard-rows
branch
from
1b81621
to
9dbea8e
Compare
fix: update tests that need adjusted rows_to_discard values
cryptonemo
force-pushed
the
update-discard-rows
branch
from
9dbea8e
to
dd54a23
Compare
| @@ -168,7 +168,7 @@ impl StoreConfig { | |||
| match branches { | |||
| 2 => std::cmp::min(max_rows_to_discard, 7), | |||
| 4 => std::cmp::min(max_rows_to_discard, 5), | |||
| _ => std::cmp::min(max_rows_to_discard, 3), | |||
| _ => std::cmp::min(max_rows_to_discard, 2), | |||
There was a problem hiding this comment.
It's not obvious to me why this change is desired or correct. Can you explain? I probably am just not visualizing the flow of when this can be called or how it is used. Let's consider the case where a user has set a default rows_to_discard of 4 (just for example). Will this expression evaluating to 2 ever cause any problems? I guess the point is that this function won't be called at all in that case. But what motivates a change in the default here and now?
There was a problem hiding this comment.
This is a method that provides a default, so setting a default (or having a pre-set default) is not related at this point. rust-fil-proofs (as of PR filecoin-project/rust-fil-proofs#1220) no longer uses this method at all, however if it did (i.e. current master), the 'default' value provided does not match the default that rust-fil-proofs defaults to, which is the main issue. Since it's no longer used, this could be not updated, but since it bit us once ...
No description provided.