Skip to content

Conversation

@anyangml
Copy link
Collaborator

This is a follow up PR of #372 . For OOD-FF datasets, we provide both PBE level and wB97MV level labels as separate datasets. We allow model to select the corresponding data path based on the model branch.

@anyangml anyangml requested review from caic99 and Copilot October 29, 2025 08:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the model instantiation workflow to support domain-specific model heads (molecules vs. materials) for models trained with OMol25 datasets. The key changes include delaying model instantiation until the specific task domain is known, enabling proper selection of model heads and DFT labels.

  • Refactored model gathering to separate parameter collection from model instantiation
  • Added support for domain-specific model heads (model_domain attribute) for molecules and materials
  • Modified the calc property from cached_property to regular property with lazy initialization and setter support

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lambench/workflow/entrypoint.py Split gather_models() into gather_model_params() and gather_model() to enable delayed model instantiation based on task domain
lambench/models/basemodel.py Added supports_omol and model_domain attributes to support domain-specific model configurations
lambench/models/ase_models.py Changed calc from cached_property to property with setter, added domain-based head selection for UMA and DP calculators, and logic to select appropriate DFT labels
lambench/tasks/base_task.py Modified test_data type to support both single Path and dict of Paths for multiple DFT labels
lambench/tasks/direct/direct_tasks.yml Added AQM task with PBE and wB97 test data paths, moved ANI and MD22 to deprecated section
lambench/metrics/utils.py Updated to use new gather_model_params() and gather_model() functions
tests/workflow/test_entrypoint.py Updated test fixture to return dict instead of DPModel instance, aligned with new parameter-based approach
tests/tasks/calculator/test_nve_md.py Simplified test fixtures by removing separate calculator fixture and accessing calculator through model
Comments suppressed due to low confidence (1)

lambench/models/ase_models.py:37

  • The documentation still describes calc as a 'cached property', but it has been changed to a regular property with lazy initialization. Update the documentation to reflect this change.
        calc (Calculator): A cached property that initializes and returns an ASE Calculator

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Oct 29, 2025

Codecov Report

❌ Patch coverage is 60.00000% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.75%. Comparing base (063ad5e) to head (bbaa30c).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
lambench/models/ase_models.py 45.83% 13 Missing ⚠️
lambench/workflow/entrypoint.py 59.09% 5 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #373      +/-   ##
==========================================
- Coverage   65.89%   65.75%   -0.14%     
==========================================
  Files          35       35              
  Lines        1589     1609      +20     
  Branches      189      194       +5     
==========================================
+ Hits         1047     1058      +11     
- Misses        500      507       +7     
- Partials       42       44       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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