Save progress#28
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the metric logging and evaluation functionality across various training scripts, ensuring that progress is saved using the last element of each metric vector rather than the explicit epoch index. It also removes the epoch parameter from evaluation functions, incorporates structured error handling in the quantum training loop, and updates the usage of the logistic function in the RBM code.
- Updated metric logging via using "end" indexing instead of explicit epoch indices.
- Removed the epoch parameter from evaluation and divergence-checking functions.
- Added error handling and resume logic in the quantum training loop.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/utils.jl | Updated _log_metrics to print the last metric value instead of by epoch. |
| src/training/train_quantum_pcd.jl | Revised evaluate and _diverged calls, added try-catch for error handling and resume functionality. |
| src/training/train_pcd.jl, train_fast_pcd.jl, train_cd.jl | Removed the epoch parameter in evaluate calls and divergence checks. |
| src/rbm.jl | Replaced _sigmoid with logistic and refactored conditional_prob_y_given_v. |
| src/qubo.jl | Averaged quantum sampling results using the provided num_evaluated_states parameter. |
| src/evaluation.jl | Removed epoch parameter in _evaluate and _diverged and replaced explicit indices with end indexing. |
| docs/make.jl | Minor syntax fix in deploydocs call. |
| README.md | Streamlined content, pointing users to documentation for getting started. |
Comments suppressed due to low confidence (1)
src/rbm.jl:165
- Ensure that the 'logistic' function is properly imported or defined, as its usage in place of _sigmoid requires validation to prevent runtime issues.
conditional_prob_h(rbm::AbstractRBM, v::Vector{<:Number}) = logistic.(rbm.b .+ rbm.W' * v)
There was a problem hiding this comment.
Pull Request Overview
This pull request updates various training and evaluation modules to improve progress tracking by replacing explicit epoch indices with usage of the latest metric values, and it refines error handling and function implementations for clarity and consistency.
- Updated metric logging and divergence checks to consistently use the latest value (using [end])
- Modified training loops (train_quantum_pcd, train_pcd, train_fast_pcd, train_cd) to remove explicit epoch parameters and incorporate robust error handling
- Replaced internal _sigmoid calls with logistic in RBM routines and improved qubo sampling and documentation from README
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/utils.jl | Changed metric printing to use the last element instead of an epoch index |
| src/training/train_quantum_pcd.jl | Updated evaluate/_diverged calls for metric progress and added error handling in loop |
| src/training/train_pcd.jl | Removed epoch parameter from evaluate calls |
| src/training/train_fast_pcd.jl | Removed epoch parameter from evaluate calls |
| src/training/train_cd.jl | Removed epoch parameter from evaluate calls |
| src/rbm.jl | Replaced _sigmoid with logistic for improved consistency |
| src/qubo.jl | Modified sampling to incorporate weighted averaging of evaluated states |
| src/evaluation.jl | Updated evaluation functions to use the last metric entry |
| docs/make.jl | Adjusted deploydocs call formatting |
| README.md | Simplified README in favor of external documentation |
Comments suppressed due to low confidence (4)
src/utils.jl:64
- [nitpick] Using the last element for metric logging removes the explicit epoch context; verify that this change aligns with the intended progress reporting strategy.
println("$metric_name: $(metrics[metric_name][end])")
src/rbm.jl:165
- Ensure that the 'logistic' function is defined or imported appropriately, as its replacement of '_sigmoid' may cause runtime errors if undefined.
conditional_prob_h(rbm::AbstractRBM, v::Vector{<:Number}) = logistic.(rbm.b .+ rbm.W' * v)
src/qubo.jl:130
- [nitpick] Verify that the weighted averaging of sampled values, calculated as 'sampled_reads / total_reads', accurately reflects the intended sampling behavior from multiple evaluated states.
for i in 1:num_evaluated_states
README.md:36
- [nitpick] Ensure that the external documentation is comprehensive enough to replace the previously detailed 'Getting started' section now removed from the README.
For more information on how to install and use the package, please refer to the [documentation](https://niteq.github.io/QARBoM.jl/dev/).
| @error "Error at epoch $epoch" exception = e | ||
| for key in keys(metrics_dict) | ||
| pop!(metrics_dict[key]) | ||
| end | ||
| break |
There was a problem hiding this comment.
[nitpick] The error handling block currently swallows exceptions after modifying the metrics; consider adding more detailed logging or alternative error propagation to avoid silent failures during training.
| @error "Error at epoch $epoch" exception = e | |
| for key in keys(metrics_dict) | |
| pop!(metrics_dict[key]) | |
| end | |
| break | |
| @error "Error at epoch $epoch" exception = e stacktrace = stacktrace(catch_backtrace()) | |
| for key in keys(metrics_dict) | |
| pop!(metrics_dict[key]) | |
| end | |
| rethrow() |
No description provided.