Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds support for configurable output directories for simulation output files and restart files. The changes introduce a new output_dir option to MeshBlockOptionsImpl with a default value of "." (current directory), and updates all file path construction throughout the output and restart loading code to prepend this directory.
Changes:
- Added
output_dirfield toMeshBlockOptionsImplwith default value "." - Updated file path construction in all output modules (restart, netcdf) and combine functions
- Refactored
load_restartfunction to return Variables map instead of taking it as a reference parameter - Updated Python bindings to expose the new
output_diroption - Fixed parameter order in Python type stub for
forwardmethod (but introduced syntax error)
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/mesh/meshblock.hpp | Added output_dir field to MeshBlockOptionsImpl with default value "." |
| src/mesh/meshblock.cpp | Updated restart initialization to prepend output_dir to restart file paths and refactored to use new load_restart API |
| src/output/restart.cpp | Updated file path construction to include output_dir |
| src/output/netcdf.cpp | Updated file path construction to include output_dir |
| src/output/combine_restart.cpp | Updated file path construction for both input and output files to include output_dir |
| src/output/combine_netcdf.cpp | Updated file path construction for both input and output files to include output_dir |
| src/input/read_restart_file.hpp | Changed load_restart signature to return Variables instead of taking reference parameter |
| src/input/read_restart_file.cpp | Refactored load_restart and helper functions to return Variables map; improved error handling with explicit checks |
| python/csrc/pymesh.cpp | Added Python binding for output_dir option |
| python/snapy/mesh.pyi | Fixed parameter order in forward method signature and corrected documentation (but missing comma causing syntax error) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def forward( | ||
| self, | ||
| vars: Dict[str, torch.Tensor] |
There was a problem hiding this comment.
Missing comma after the first parameter in the function signature. The line vars: Dict[str, torch.Tensor] should be followed by a comma before the next parameter dt: float.
| vars: Dict[str, torch.Tensor] | |
| vars: Dict[str, torch.Tensor], |
|
🎉 Released v1.3.0! What's Changed
Full Changelog: v1.2.9...v1.3.0 |
No description provided.