feat: add standalone partition assignment operation #2556
Conversation
|
|
||
| dataset: LanceDataset | ||
| the dataset containing the data | ||
| column: str |
There was a problem hiding this comment.
This IndicesBuilder is vector only?
There was a problem hiding this comment.
Yes, can probably rename in future PR.
| row_id: uint64 | ||
| partition: uint32 | ||
|
|
||
| Note: There is no advantage to separately computing the partition assignment |
There was a problem hiding this comment.
Maybe it is also useful for checkpoint purpose.
There was a problem hiding this comment.
Hmm...perhaps. Though I think a better way to checkpoint the "transform vectors" stage will be to break up the work by fragments.
python/python/lance/indices.py
Outdated
| def assign_ivf_partitions( | ||
| self, | ||
| ivf_model: IvfModel, | ||
| dst_dataset_uri: Optional[str], |
There was a problem hiding this comment.
dst_dataset_uri => output_uri? we use less abbreviation in python SDKs.
Btw, should this be a dict arg, i.e., output_uri = None if we expect that user won't set it for most of the time?
There was a problem hiding this comment.
I can change to output_uri. I could set it to output_uri = None (which would use a random tmp path?). I think that would be confusing though since the other methods are going to need paths. E.g. if a user is going down the "piece by piece" standalone route then they want to know and be aware of all the temp files because that is what they are trying to do.
There was a problem hiding this comment.
Ok, I came around. I changed to output_uri and default to None.
westonpace
force-pushed
the
feat/standalone-partition-assignment
branch
from
f446aa8
to
81759a6
Compare
This also fixes up the tqdm progress bar for partition assignment so that it has a definite end. This also makes one small tweak to the indices builder, moving the column argument into the builder constructor, since this argument will be shared by all methods in the class.