Conversation
Summary of ChangesHello @lazarusA, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request lays the groundwork for robust configuration management by integrating YAML file support. It introduces new utilities to handle the loading and saving of model and training parameters, enhancing the ability to reproduce experiments and manage complex settings. The changes also ensure that all training arguments are captured and stored with the training results, providing a comprehensive record of each run. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces YAML configuration capabilities by adding OrderedCollections and YAML packages, and new functions for loading and saving hybrid model configurations. It also updates the TrainResults struct to include train_args, allowing for better tracking of training parameters. The changes are well-contained and improve the configurability and reproducibility of the models.
| NCDatasets = "85f8d34a-cbdd-5861-8df4-14fed0d494ab" | ||
| NamedDims = "356022a1-0364-5f58-8944-0da4b18d706f" | ||
| OptimizationOptimisers = "42dfb2eb-d2b4-4451-abcd-913932933ac1" | ||
| OrderedCollections = "bac558e1-5e72-5ebc-8fee-abe8a469f55d" |
There was a problem hiding this comment.
The OrderedCollections dependency has been added. Please ensure that this package is necessary for the YAML functionality and that its inclusion does not introduce any unnecessary overhead or conflicts with existing dependencies.
| Static = "aedffcd0-7271-4cad-89d0-dc628f76c6d3" | ||
| Statistics = "10745b16-79ce-11e8-11f9-7d13ad32a3b2" | ||
| StyledStrings = "f489334b-da3d-4c2e-b8f0-e476e12c162b" | ||
| YAML = "ddb6d928-2868-570f-bddf-ab3f9cf99eb6" |
There was a problem hiding this comment.
The YAML dependency has been added. This is a core dependency for the new configuration features. Please confirm that the chosen version 0.4.16 is compatible with other packages and the Julia version.
| NCDatasets = "0.14.8" | ||
| NamedDims = "1.2.3" | ||
| OptimizationOptimisers = "0.3.7" | ||
| OrderedCollections = "1.8.1" |
There was a problem hiding this comment.
The OrderedCollections compatibility entry has been added. Ensure that the specified version range 1.8.1 is accurate and covers the required functionality without introducing breaking changes.
| Static = "1.3.1" | ||
| Statistics = "1" | ||
| StyledStrings = "1.0.3, 1.11.0" | ||
| YAML = "0.4.16" |
There was a problem hiding this comment.
The YAML compatibility entry has been added. Please verify that the version 0.4.16 is the correct and stable version to use for this project.
src/train.jl
Outdated
| st | ||
| best_epoch | ||
| best_loss | ||
| train_args |
There was a problem hiding this comment.
Adding train_args to TrainResults is a good improvement for reproducibility, as it allows all training parameters to be stored with the results. This is a high-value addition for debugging and understanding past training runs.
src/train.jl
Outdated
| best_epoch, | ||
| best_agg_loss | ||
| best_agg_loss, | ||
| (; nepochs, batchsize, opt, patience, autodiff_backend, return_gradients, array_type, training_loss, loss_types, extra_loss, agg, train_from, random_seed, file_name, hybrid_name, return_model, monitor_names, folder_to_save, plotting, show_progress, yscale) |
There was a problem hiding this comment.
The train_args tuple is being constructed with a comprehensive list of training parameters. This ensures that all relevant arguments are captured and stored in TrainResults, which is excellent for traceability and reproducibility. Consider if any other kwargs passed to train should also be explicitly captured here for completeness.
src/utils/config_yaml.jl
Outdated
| function get_hybrid_config(hm::HybridModel) | ||
| hm_config = Dict{String,Any}() | ||
| for field in fieldnames(typeof(hm)) | ||
| hm_config[string(field)] = getfield(hm, field) | ||
| end | ||
| return hm_config | ||
| end |
There was a problem hiding this comment.
The get_hybrid_config function dynamically extracts all fields from a HybridModel instance into a dictionary. This is a flexible approach for serializing model configurations. Ensure that all fields are suitable for direct serialization to YAML.
src/utils/config_yaml.jl
Outdated
| function get_train_config(train_args::TrainResults) | ||
| train_config = Dict{String,Any}() | ||
| for field in fieldnames(typeof(train_args)) | ||
| train_config[string(field)] = getfield(train_args, field) | ||
| end | ||
| return train_config | ||
| end No newline at end of file |
There was a problem hiding this comment.
Similar to get_hybrid_config, get_train_config extracts all fields from TrainResults. This is crucial for saving the complete training state and arguments, enhancing reproducibility. Consider if any fields within TrainResults might contain non-serializable objects that would cause issues when writing to YAML.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #231 +/- ##
==========================================
+ Coverage 50.31% 50.84% +0.53%
==========================================
Files 27 28 +1
Lines 1932 1957 +25
==========================================
+ Hits 972 995 +23
- Misses 960 962 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
closes #230
still work in progress. But almost there.