Skip to content

feat: #220 add index creation logic#221

Merged
fedorgrab merged 1 commit into
mainfrom
fg/issue-220
Jul 1, 2026
Merged

feat: #220 add index creation logic#221
fedorgrab merged 1 commit into
mainfrom
fg/issue-220

Conversation

@fedorgrab

@fedorgrab fedorgrab commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

This pr introduces the script that creates vsindex. It was in cellarium-cas-workflows previously, but I wanted to move it here, so it can live outside of the pipeline and be just imported there.

Closes #220

Comment thread tests/unit/scripts/test_create_vsindex.py Fixed
Comment thread tests/unit/scripts/test_create_vsindex.py Fixed
Comment thread tests/unit/scripts/test_create_vsindex.py Fixed
Comment thread tests/unit/scripts/test_create_vsindex.py Fixed
Comment thread tests/unit/scripts/test_create_vsindex.py Fixed
Comment thread tests/unit/scripts/test_create_vsindex.py Fixed
Comment thread tests/unit/scripts/test_create_vsindex.py Fixed
Comment thread tests/unit/scripts/test_create_vsindex.py Fixed
Comment thread tests/unit/scripts/test_create_vsindex.py Fixed
Comment thread tests/unit/scripts/test_create_vsindex.py Fixed
logging.info(f"Creating IVF_FLAT index: {partitions} partitions, ~{estimated_total} total vectors")

index = vs.ingest(
index_type="IVF_FLAT",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never, I have no idea what this thing is. Maybe I should learn more about it!

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not necessary now, you can make a new issue! just wondering if it might have some advantages / disadvantages compared to IVF_FLAT. but IVF_FLAT is working fine currently.

Comment on lines +143 to +145
for i in range(total_batches):
df = _read_csv_gz(f"{embeddings_prefix}/batch_{i}.csv.gz")
dfs.append(df)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know if you need a speedup here: you may not... but if you do, multiprocessing / multithreading (I forget which) can make this kind of multi-csv-read pretty fast

Might not be worth it at all if this is not a bottleneck

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've run into situations in the past where sequential reading of these was like 40 mins and parallel reading was like 5 mins

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oooooh nevermind, you're breaking this loop after a certain size

Comment thread cellarium/cas_backend/scripts/create_vsindex.py

@sjfleming sjfleming left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it looks good. I have just a couple comments and questions. I had initially recommended some kind of parallel .csv.gz file reading, but I don't know if it actually makes sense in this case, since you're only reading a certain smaller number of cells upfront to create the index, and then you're (necessarily sequentially) ingesting more cells into the index after that.

@sjfleming sjfleming self-requested a review July 1, 2026 18:56

@sjfleming sjfleming left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@sonarqubecloud

sonarqubecloud Bot commented Jul 1, 2026

Copy link
Copy Markdown

@fedorgrab fedorgrab merged commit 00408d8 into main Jul 1, 2026
3 checks passed
@fedorgrab fedorgrab deleted the fg/issue-220 branch July 1, 2026 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Transfer index creation logic from cellarium-cas-workflows

3 participants