-
Notifications
You must be signed in to change notification settings - Fork 4
fix(notebook): recompute projections on every generate #404
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
7d80a0d
3d9d670
3c4b9f4
82b6dbb
af4ffca
5583733
ee90470
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,7 +57,10 @@ | |
| " PipelineConfig,\n", | ||
| " ReducerParams,\n", | ||
| " ReductionPipeline,\n", | ||
| " _embedding_cache_path,\n", | ||
| " _input_cache_dir,\n", | ||
| " parse_methods_arg,\n", | ||
| " _query_fasta_cache_path,\n", | ||
| ")" | ||
| ] | ||
| }, | ||
|
|
@@ -759,8 +762,8 @@ | |
| "\n", | ||
| " out_dir = Path(\"output\")\n", | ||
| " out_dir.mkdir(exist_ok=True)\n", | ||
| " cache_dir = out_dir / \"tmp\"\n", | ||
| " cache_dir.mkdir(exist_ok=True)\n", | ||
| " cache_root = out_dir / \"tmp\"\n", | ||
| " cache_root.mkdir(exist_ok=True)\n", | ||
| " output_path = out_dir / \"data.parquetbundle\"\n", | ||
| "\n", | ||
| " step_html = HTML(value=\"<b>Step 1/4: Loading embeddings...</b>\")\n", | ||
|
|
@@ -776,7 +779,7 @@ | |
| " print(\"Select at least one embedder.\")\n", | ||
| " return\n", | ||
| " step_html.value = \"<b>Step 1/6: Downloading FASTA...</b>\"\n", | ||
| " fasta_cache = cache_dir / \"sequences.fasta\"\n", | ||
| " fasta_cache = _query_fasta_cache_path(cache_root, inp[\"query\"])\n", | ||
| " if fasta_cache.exists() and fasta_cache.stat().st_size > 0:\n", | ||
| " from protspace.data.loaders.query import (\n", | ||
| " extract_identifiers_from_fasta,\n", | ||
|
|
@@ -792,6 +795,8 @@ | |
| " if not headers:\n", | ||
| " print(f\"No sequences found for query: {inp['query']}\")\n", | ||
| " return\n", | ||
| " cache_dir = _input_cache_dir(cache_root, fasta_path)\n", | ||
| " cache_dir.mkdir(parents=True, exist_ok=True)\n", | ||
| " backend, _emb_cfg = _resolve_backend_and_config()\n", | ||
| " embs = _drop_incompatible(embs, backend)\n", | ||
| " if not embs:\n", | ||
|
|
@@ -804,7 +809,7 @@ | |
| " emb_name,\n", | ||
| " backend=backend,\n", | ||
| " embed_config=_emb_cfg,\n", | ||
| " embedding_cache=cache_dir / f\"{emb_name}.h5\",\n", | ||
| " embedding_cache=_embedding_cache_path(cache_dir, emb_name, backend),\n", | ||
| " )\n", | ||
| " emb_set.fasta_path = fasta_path\n", | ||
| " embedding_sets.append(emb_set)\n", | ||
|
|
@@ -813,6 +818,9 @@ | |
| " if not embs:\n", | ||
| " print(\"Select at least one embedder.\")\n", | ||
| " return\n", | ||
| " fasta_path = Path(inp[\"path\"])\n", | ||
| " cache_dir = _input_cache_dir(cache_root, fasta_path)\n", | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Scope the embedding cache by backend This content-owned directory is later paired with only
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implemented in |
||
| " cache_dir.mkdir(parents=True, exist_ok=True)\n", | ||
| " backend, _emb_cfg = _resolve_backend_and_config()\n", | ||
| " embs = _drop_incompatible(embs, backend)\n", | ||
| " if not embs:\n", | ||
|
|
@@ -821,23 +829,25 @@ | |
| " for emb_name in embs:\n", | ||
| " step_html.value = f\"<b>Step 1/5: Computing {emb_name} embeddings ({backend})...</b>\"\n", | ||
| " emb_set = embed_fasta(\n", | ||
| " Path(inp[\"path\"]),\n", | ||
| " fasta_path,\n", | ||
| " emb_name,\n", | ||
| " backend=backend,\n", | ||
| " embed_config=_emb_cfg,\n", | ||
| " embedding_cache=cache_dir / f\"{emb_name}.h5\",\n", | ||
| " embedding_cache=_embedding_cache_path(cache_dir, emb_name, backend),\n", | ||
| " )\n", | ||
| " emb_set.fasta_path = Path(inp[\"path\"])\n", | ||
| " emb_set.fasta_path = fasta_path\n", | ||
| " embedding_sets.append(emb_set)\n", | ||
| " else:\n", | ||
| " h5_path = Path(inp[\"path\"])\n", | ||
| " cache_dir = _input_cache_dir(cache_root, h5_path)\n", | ||
| " cache_dir.mkdir(parents=True, exist_ok=True)\n", | ||
| " name_override = inp.get(\"name\")\n", | ||
| " emb_set = load_h5([h5_path], name_override=name_override)\n", | ||
| " embedding_sets.append(emb_set)\n", | ||
| "\n", | ||
| " n_proteins = len(embedding_sets[0].headers)\n", | ||
| "\n", | ||
| " # Build pipeline with caching enabled\n", | ||
| " # Build pipeline with non-projection caching enabled\n", | ||
| " reducer_params = ReducerParams(\n", | ||
| " n_neighbors=pw[\"n_neighbors\"].value,\n", | ||
| " min_dist=pw[\"min_dist\"].value,\n", | ||
|
|
@@ -852,6 +862,7 @@ | |
| " bundled=True,\n", | ||
| " keep_tmp=True,\n", | ||
| " intermediate_dir=cache_dir,\n", | ||
| " refetch_stages=frozenset({\"projections\"}),\n", | ||
| " annotations=ann,\n", | ||
| " reducer_params=reducer_params,\n", | ||
| " stats=compute_stats_cb.value,\n", | ||
|
|
@@ -860,7 +871,9 @@ | |
| "\n", | ||
| " # Step 2: Annotations (cached after first run)\n", | ||
| " step_html.value = \"<b>Step 2/4: Fetching annotations...</b>\"\n", | ||
| " metadata = pipeline._fetch_annotations(embedding_sets[0].headers)\n", | ||
| " metadata = pipeline._fetch_annotations(\n", | ||
| " embedding_sets[0].headers, embedding_sets\n", | ||
| " )\n", | ||
| "\n", | ||
| " # Step 3: Dimensionality reduction\n", | ||
| " step_html.value = \"<b>Step 3/4: Reducing dimensions...</b>\"\n", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[P2] Publish query FASTA caches atomically
The new query-addressed path is still accepted solely when it exists and has nonzero size.
query_uniprotwritessave_todirectly, so interruption during gzip extraction can leave a truncated-but-nonempty FASTA; the next Generate extracts whatever headers are present and permanently treats that subset as the full query result. I reproduced acceptance with a one-record partial file at this path. Please write to a temporary sibling and atomically rename only after extraction succeeds (or persist equivalent completion metadata), and cover partial-cache recovery.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented in
82b6dbb2.query_uniprotnow extracts into a temporary sibling, verifies the complete write and ordered identifiers against the downloaded gzip, atomically replaces the query-addressed final path only after validation, and cleans compressed/staged artifacts infinally. The interruption regression writes partial bytes and proves neither the final cache nor a staged sibling survives; successful publication and same-query path reuse are covered. Verification: focused suite21 passed; full non-slow Python suite801 passed, 6 deselected; Ruff, notebook parse, strict OpenSpec, andpnpm precommitall passed.