Flow solver is now almost fully refactored and optimised #61
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
since this function is no longer used
removed comments and code I intend to deprecate
removed comments and code I intend to deprecate
…ive_impl_part also refactored second_projection.divergence_nodes. this commit removes more unnecessary code and make everything consistent with mem as an input argument
not all operators will be used, but might be worth keeping for future sake
but this appears to be really slow, as the stencil is never cached.
this seems to be substantially faster as it can be cached
and reflective of the current directory structure
fixed minor bug with set_boundary on rho
ray-chew
added
enhancement
There was a problem hiding this comment.
Pull Request Overview
This PR refactors and optimises crucial bottlenecks in the flow solver while restructuring the codebase for improved clarity and maintenance. Key changes include removal of legacy functions, renaming and consolidation of timestep and flux routines, and updating module dependencies from mpv to npf.
- Removed the obsolete synchronise_variables function in the EOS module.
- Renamed functions and updated boundary handling across CFD, advection, and data assimilation modules.
- Updated test and documentation references to reflect the new module structure.
Reviewed Changes
Copilot reviewed 61 out of 61 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/pybella/flow_solver/physics/eos.py | Removed unused legacy functions. |
| src/pybella/flow_solver/physics/cfl.py | Updated function calls and parameter formatting for clarity. |
| src/pybella/flow_solver/numerics/implicit_euler.py | New implementations with a potential issue in 3D system preparation. |
| src/pybella/flow_solver/numerics/explicit_euler.py | Refactored explicit Euler step with minor style updates. |
| src/pybella/flow_solver/numerics/explicit_advection/* | Renamed and restructured advection flux and recovery functions. |
| src/pybella/flow_solver/discretisation/time_update.py | Adapted to use new npf variables and updated boundary calls. |
| src/pybella/data_assimilation/* | Updated dependency references to use the new boundary modules. |
| docs/* | Updated API and documentation to match module refactorings. |
Comments suppressed due to low confidence (2)
src/pybella/flow_solver/numerics/explicit_advection/compute_advection.py:8
- [nitpick] The function name 'strange_splitting' is ambiguous and does not clearly describe its behavior. Consider renaming it to a more descriptive name, such as 'advect' or 'explicit_splitting', to improve code clarity.
def strange_splitting(mem, ud, dt, odd, label, writer=None):
src/pybella/flow_solver/numerics/implicit_euler.py:219
- In _prepare_3d_system, diag_inv is explicitly set to None, which may indicate a missing preconditioner for the 3D case. Please verify whether a computed diag_inv is required to ensure the solver's correctness.
diag_inv = None # TODO: Verify if this should be computed for 3D
|
Yet another massive PR. I am going to merge this, bypassing review requirements, after all tests have passed. |
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some stragglers have not yet been touched, see #57. But all crucial bottleneck functions have been refactored and optimised. The codebase has also been extensively restructured. This PR will prepare the stage for the plans in roadmap #60.