Use dict for Index serialization #258
Conversation
|
@yurymalkov PR is ready for the review. One item to potentially look into: pybind11 complained when I tried returning dict as object's state, so I wrapped the parameter dict into tuple (of length one). I did not give it too much thought, tbh, so it might still be possible to avoid this. |
There was a problem hiding this comment.
@dbespalov Thanks for an awesome contribution!
One cosmetic change I would suggest is to extend the assert messages, so in case of a failure a user would better understand what has happened.
| assert_true(appr_alg->offsetLevel0_ == d["offset_level0"].cast<size_t>(), "Invalid value of offsetLevel0_ "); | ||
| assert_true(appr_alg->max_elements_ == d["max_elements"].cast<size_t>(), "Invalid value of max_elements_ "); | ||
|
|
||
| appr_alg->cur_element_count = d["cur_element_count"].cast<size_t>(); | ||
|
|
||
| appr_alg->cur_element_count = t[2].cast<size_t>(); | ||
| assert_true(appr_alg->size_data_per_element_ == d["size_data_per_element"].cast<size_t>(), "Invalid value of size_data_per_element_ "); | ||
| assert_true(appr_alg->label_offset_ == d["label_offset"].cast<size_t>(), "Invalid value of label_offset_ "); | ||
| assert_true(appr_alg->offsetData_ == d["offset_data"].cast<size_t>(), "Invalid value of offsetData_ "); | ||
|
|
||
| assert_true(appr_alg->size_data_per_element_ == t[3].cast<size_t>(), "Invalid value of size_data_per_element_ "); | ||
| assert_true(appr_alg->label_offset_ == t[4].cast<size_t>(), "Invalid value of label_offset_ "); | ||
| assert_true(appr_alg->offsetData_ == t[5].cast<size_t>(), "Invalid value of offsetData_ "); | ||
| appr_alg->maxlevel_ = d["max_level"].cast<int>(); | ||
| appr_alg->enterpoint_node_ = d["enterpoint_node"].cast<hnswlib::tableint>(); | ||
|
|
||
| appr_alg->maxlevel_ = t[6].cast<int>(); | ||
| appr_alg->enterpoint_node_ = t[7].cast<hnswlib::tableint>(); | ||
| assert_true(appr_alg->maxM_ == d["max_M"].cast<size_t>(), "Invalid value of maxM_ "); | ||
| assert_true(appr_alg->maxM0_ == d["max_M0"].cast<size_t>(), "Invalid value of maxM0_ "); | ||
| assert_true(appr_alg->M_ == d["M"].cast<size_t>(), "Invalid value of M_ "); | ||
| assert_true(appr_alg->mult_ == d["mult"].cast<double>(), "Invalid value of mult_ "); | ||
| assert_true(appr_alg->ef_construction_ == d["ef_construction"].cast<size_t>(), "Invalid value of ef_construction_ "); |
There was a problem hiding this comment.
I think the messages might confuse the user. I would add something like "Unpickle error:...".
There was a problem hiding this comment.
changed the error message to "Unpickle error"
|
@yurymalkov good point, changed the error message. Thank you! |
|
Great! Thank you so much! |
@yurymalkov @dyashuni this PR is still a WIP and untested. I should be a able to finish it tomorrow