-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor evaluators #107
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?
Refactor evaluators #107
Conversation
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.
Pull request overview
This PR refactors the evaluation system to be more flexible and config-driven, removing the Julia integration, adding new model architectures (Sequential, GPSTransformer), and expanding test coverage. Key changes include:
- Renamed evaluators to
Evaluator,Validator,Testerwith configurable monitor tasks - Made early stopping optional and updated Trainer schema to support both constructor and evaluator config formats
- Added comprehensive tests for evaluators, sequential models, and GPS transformers
- Removed Julia worker integration and tests
- Updated dataset handling for Zarr stores without root groups
Reviewed changes
Copilot reviewed 34 out of 36 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
evaluate.py |
Refactored evaluators with DataFrame-based results and configurable monitor tasks |
train.py, train_ddp.py |
Updated trainer to use new evaluator names and optional early stopping |
test_evaluate.py |
Added comprehensive evaluator tests with custom monitors |
test_sequential.py, test_gps_transformer.py |
New tests for Sequential and GPSTransformer models |
test_trainer.py |
Updated tests for default evaluators and trainer configuration |
test_tune.py |
Refactored fixture to use yaml_config fixture |
conftest.py |
Replaced Julia-based fixtures with pure Python data generation |
config_utils.py |
Added support for list sweeps and numeric string conversion |
dataset_base.py, dataset_ondisk.py |
Updated to handle Zarr stores without root groups |
gnn_model.py |
Fixed pooling layer validation logic |
sequential.py, gps_transformer.py |
New configurable model architectures |
models/__init__.py |
Exported new Sequential model |
utils.py |
Added maybe_number utility |
pyproject.toml |
Removed juliacall, added huggingface-hub |
| Documentation files | Updated guides and API docs for new evaluator system |
| Config files | Added example configs for training |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| original_weights = [param.clone() for param in trainer.model.parameters()] | ||
|
|
||
| training_data, valid_data = trainer.run_training( |
Copilot
AI
Dec 29, 2025
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.
Variable training_data is not used.
|
|
||
| original_weights = [param.clone() for param in trainer.model.parameters()] | ||
|
|
||
| training_data, valid_data = trainer.run_training( |
Copilot
AI
Dec 29, 2025
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.
Variable valid_data is not used.
depends on #101