Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds grid refinement and coarsening utilities to the codebase, implementing conservative interpolation methods for multi-resolution grid operations. Additionally, it refactors the implicit hydro options implementation by consolidating code and converting the type field from a stored property to a computed method.
Changes:
- Added
conservative_refineandconservative_coarsenfunctions using PyTorch interpolation - Consolidated implicit hydro options code from a separate file into the main implementation file
- Converted
ImplicitOptionsImpl::typefrom a stored property to a computed method based on scheme value
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/refine.hpp | Declares new public API for conservative grid refinement and coarsening |
| src/utils/refine.cpp | Implements conservative interpolation using multi-step PyTorch operations |
| tests/test_refine.cpp | Adds parameterized test to verify refine/coarsen operations are inverse operations |
| tests/CMakeLists.txt | Registers the new refine test in the build system |
| src/implicit/implicit_options.cpp | Removed - functionality moved to implicit_hydro.cpp |
| src/implicit/implicit_hydro.hpp | Changed type from stored property to method declaration |
| src/implicit/implicit_hydro.cpp | Added YAML parsing and type() method implementation from deleted file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| torch::Tensor conservative_refine(torch::Tensor x); |
There was a problem hiding this comment.
The public API functions conservative_refine and conservative_coarsen are missing documentation comments. Similar utility functions in this codebase (e.g., pull_neighbors2/3/4 in src/utils/pull_neighbors.hpp) include Doxygen-style documentation that explains the purpose, parameters, and return values. Consider adding documentation to describe what these functions do, their input tensor requirements (e.g., expected dimensions), and what the output tensor represents.
| torch::Tensor conservative_refine(torch::Tensor x); | |
| /** | |
| * @brief Perform conservative refinement of a tensor field. | |
| * | |
| * This utility takes values defined on a coarse resolution grid and produces | |
| * a refined representation while conserving the total quantity represented | |
| * by @p x along the refined dimensions. | |
| * | |
| * The input is typically a tensor of shape (batch, channels, spatial...), | |
| * where the trailing dimensions correspond to spatial axes that will be | |
| * refined. The exact refinement scheme and resulting shape depend on the | |
| * implementation, but the operation is designed to be conservative | |
| * (i.e., the sum over corresponding regions is preserved). | |
| * | |
| * @param x Input tensor containing cell-averaged or cell-integrated values | |
| * on a coarse grid. | |
| * @return A tensor defined on a refined grid, with values adjusted so that | |
| * the total quantity is conserved with respect to @p x. | |
| */ | |
| torch::Tensor conservative_refine(torch::Tensor x); | |
| /** | |
| * @brief Perform conservative coarsening of a tensor field. | |
| * | |
| * This utility takes values defined on a fine resolution grid and produces | |
| * a coarsened representation while conserving the total quantity represented | |
| * by @p x along the coarsened dimensions. | |
| * | |
| * The input is typically a tensor of shape (batch, channels, spatial...), | |
| * where the trailing dimensions correspond to spatial axes that will be | |
| * coarsened (e.g., by aggregating neighboring cells). The exact coarsening | |
| * scheme and resulting shape depend on the implementation, but the operation | |
| * is designed to be conservative (i.e., the sum over aggregated regions is | |
| * preserved). | |
| * | |
| * @param x Input tensor containing cell-averaged or cell-integrated values | |
| * on a fine grid. | |
| * @return A tensor defined on a coarser grid, with values adjusted so that | |
| * the total quantity is conserved with respect to @p x. | |
| */ |
| break; | ||
| case 1: | ||
| return "vic-partial"; | ||
| break; | ||
| case 9: | ||
| return "vic-full"; | ||
| break; |
There was a problem hiding this comment.
The break statements after return statements are unreachable code. Since each case returns a value, the break statements on lines 34, 37, and 40 will never be executed and should be removed.
| break; | |
| case 1: | |
| return "vic-partial"; | |
| break; | |
| case 9: | |
| return "vic-full"; | |
| break; | |
| case 1: | |
| return "vic-partial"; | |
| case 9: | |
| return "vic-full"; |
| .scale_factor(std::vector<double>({2.0, 2.0, 1.0})) | ||
| .mode(torch::kArea); | ||
|
|
||
| // bilinear refine |
There was a problem hiding this comment.
The comment says "bilinear refine" but the interpolation mode is set to torch::kTrilinear on line 8. The comment should be updated to "trilinear refine" to accurately reflect the interpolation method being used.
| // bilinear refine | |
| // trilinear refine |
|
🎉 Released v1.2.5! What's Changed
Full Changelog: v1.2.4...v1.2.5 |
1 participant
No description provided.