Skip to content

feat: add provgigapath model#13

Open
vojtech-cifka wants to merge 1 commit into
masterfrom
feature/embeddings-prov-gigapath
Open

feat: add provgigapath model#13
vojtech-cifka wants to merge 1 commit into
masterfrom
feature/embeddings-prov-gigapath

Conversation

@vojtech-cifka
Copy link
Copy Markdown
Collaborator

@vojtech-cifka vojtech-cifka commented May 15, 2026

Adds the config for running the provgigapath using the new SDK.

Summary by CodeRabbit

Chores

  • Added preprocessing configuration support for the prov-gigapath model.

Review Change Stack

@vojtech-cifka vojtech-cifka requested review from Adames4 and vejtek May 15, 2026 22:47
@vojtech-cifka vojtech-cifka self-assigned this May 15, 2026
@vojtech-cifka vojtech-cifka requested a review from a team May 15, 2026 22:47
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

📝 Walkthrough

Walkthrough

A new YAML configuration file for preprocessing embeddings is added to the experiment configuration directory. The file defines the prov-gigapath model with global package scope and Hydra-style defaults that map the dataset path to /data.

Changes

Prov-GigaPath Embedding Configuration

Layer / File(s) Summary
Embedding configuration file
configs/experiment/preprocessing/embeddings_prov_gigapath_05mpp.yaml
New YAML configuration declares global package scope, sets Hydra defaults to resolve dataset paths under /data, and specifies model: prov-gigapath.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

A config takes shape, seven lines strong,
With prov-gigapath paths all along,
Hydra defaults guide the way,
New embeddings come to play. 🐇✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add provgigapath model' clearly and concisely describes the main change—adding configuration for the prov-gigapath model, which aligns with the pull request's objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/embeddings-prov-gigapath

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new configuration file for preprocessing embeddings using the prov-gigapath model. Feedback was provided regarding a naming inconsistency between the model identifier and the filename, as well as a missing trailing newline in the configuration file.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
configs/experiment/preprocessing/embeddings_prov_gigapath_05mpp.yaml (1)

1-7: 💤 Low value

Consider adding explicit resolution parameter for consistency with other preprocessing configs.

The filename includes 05mpp (0.5 microns per pixel), but this resolution is not explicitly set in the configuration. While other preprocessing configs (tiling_05mpp.yaml, tissue_masks_mpp2.yaml, etc.) explicitly define mpp parameters, the embedding configs use only the model name. Adding an explicit mpp: 0.5 would improve clarity and maintain consistency across preprocessing configurations.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@configs/experiment/preprocessing/embeddings_prov_gigapath_05mpp.yaml` around
lines 1 - 7, The config is missing an explicit resolution setting; update the
YAML by adding an mpp: 0.5 entry to this preprocessing config so it matches
other files that declare microns-per-pixel explicitly (e.g., alongside the
existing defaults block or near model: prov-gigapath) to make resolution
unambiguous and consistent with tiling_05mpp and related configs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@configs/experiment/preprocessing/embeddings_prov_gigapath_05mpp.yaml`:
- Around line 1-7: The config is missing an explicit resolution setting; update
the YAML by adding an mpp: 0.5 entry to this preprocessing config so it matches
other files that declare microns-per-pixel explicitly (e.g., alongside the
existing defaults block or near model: prov-gigapath) to make resolution
unambiguous and consistent with tiling_05mpp and related configs.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 824c3782-6424-4b0e-b5e9-ff01245c1de7

📥 Commits

Reviewing files that changed from the base of the PR and between 727aba5 and 1f15e03.

📒 Files selected for processing (1)
  • configs/experiment/preprocessing/embeddings_prov_gigapath_05mpp.yaml

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.

2 participants