fix: change default rows_to_discard for oct trees #1165
Conversation
|
An alternative to filecoin-project/merkletree#88, giving this library fuller control over the values going forward. |
storage-proofs/core/src/settings.rs
Outdated
| @@ -21,7 +21,7 @@ pub struct Settings { | |||
| pub column_write_batch_size: u32, | |||
| pub use_gpu_tree_builder: bool, | |||
| pub max_gpu_tree_batch_size: u32, | |||
| pub rows_to_discard: u32, | |||
| pub oct_rows_to_discard: u32, | |||
There was a problem hiding this comment.
I think we don't need to expose the arity here, since we use oct everywhere we discard tree rows.
| 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, rows_to_discard), | ||
| } |
There was a problem hiding this comment.
Along with a single rows_to_discard default, we could probably get rid of this match — unless it simplifies testing.
| // environment setting (FIL_PROOFS_ROWS_TO_DISCARD). If this | ||
| // value is specified, no checking is done on it and it may | ||
| // A default 'rows_to_discard' value will be chosen for tree_r_last, unless the user overrides this value via the | ||
| // environment setting (FIL_PROOFS_OCT_ROWS_TO_DISCARD). If this value is specified, no checking is done on it and it may |
There was a problem hiding this comment.
It's still not clear to me whether this is converging on what I think the right answer should be:
- If nothing is specified by 'settings', default to discarding zero rows.
- If a setting is specified by the user (env var or config file), use that.
- If nothing is specified by the user, use a default hard-coded in settings.
There was a problem hiding this comment.
This doesn't address the first case at all, as something is always specified by the settings (either the default value, or a user provided value). The last 2 cases should be satisfied.
feat: move default value from merkle_light to internal
cryptonemo
force-pushed
the
improve-rows-to-discard
branch
from
470bb7f
to
204600f
Compare
feat: move default value from merkle_light to internal