Conversation
will flesh it out again as flow solver is refactored
this is to ensure that all enumerators are visible to src.pybella.utils.user_data
…erators and slices modules
all tests passed
consider moving more things to the cache; see the TBDs
reworked numerical_flux.recompute_advective_fluxes function too; all tests passed.
also optimised the calls to var.primitives
now over 50% of the runtime for the lamb wave test is for the precompilation
this can be cleaned eventually
moved it to boundary.py
I am still allocating on the fly but this is a performance hit I am willing to take for now
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the flow-solver and blending interfaces to unify function signatures around a central mem (simulation state) object, standardizes imports, and introduces a FlowSolverCache for performance. Key changes include:
- Standardized API: functions now accept
memandudparameters instead of multiple positional arguments. - Performance enhancement: added
FlowSolverCacheto cache recovery and velocity arrays. - Cleanup: updated import paths, removed deprecated modules, and split large functions into smaller helpers.
Reviewed Changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/pybella/interfaces/dynamics_blending/schemes.py | Updated blending function signatures to use mem and ud |
| src/pybella/flow_solver/utils/variable.py | Refactored primitive calculations and added FlowSolverCache |
| src/pybella/flow_solver/utils/prepare.py | Adjusted imports and injected FlowSolverCache into simulation state |
| src/pybella/flow_solver/utils/options.py | Removed deprecated options definitions |
| src/pybella/flow_solver/utils/boundary.py | Updated imports and added scale_wall_node_values helper |
| src/pybella/flow_solver/physics/low_mach/laplacian.py | Fixed relative import path for options |
| src/pybella/flow_solver/physics/hydrostatics.py | Cleaned up array concatenations and formatting |
| src/pybella/flow_solver/physics/gas_dynamics/recovery.py | Major refactor: decoupled functions, added caching hooks |
| src/pybella/flow_solver/physics/gas_dynamics/numerical_flux.py | Refactored flux logic with utility kernels and njit |
| src/pybella/flow_solver/physics/gas_dynamics/explicit.py | Simplified advect and explicit_step_and_flux |
| src/pybella/flow_solver/physics/gas_dynamics/cfl.py | Split CFL logic into helper functions |
| src/pybella/flow_solver/discretisation/time_update.py | Unified do() signature, updated calls, but missing logging import |
| src/pybella/flow_solver/discretisation/grid.py | Fixed import path for options |
| src/pybella/data_assimilation/utils.py | Fixed import paths |
| src/pybella/data_assimilation/letkf.py | Fixed import paths |
| src/pybella/data_assimilation/analysis.py | Adjusted unpacking of mem tuple |
| src/pybella/main.py | Updated time_update invocation, introduced unused prepare import |
| run_scripts/test_suite.py | Trimmed trailing whitespace |
| README.md | Updated CI badge to deploy.yml workflow |
| .coveragerc | Updated omit paths for new package structure |
Comments suppressed due to low confidence (3)
src/pybella/interfaces/dynamics_blending/schemes.py:585
- The call to do_psinc_to_comp_conv now passes ud twice due to the updated signature; adjust the arguments to (mem, ud, bld, label, writer, step, tout) so they match the new definition.
do_psinc_to_comp_conv(
mem,
ud,
bld,
ud,
label,
writer,
step,
t + dt,
)
src/pybella/flow_solver/discretisation/time_update.py:72
- The
loggingmodule is used here but not imported; addimport loggingat the top of this file.
logging.info(f"step = {mem.time.step}, window_step = {mem.time.window_step}")
src/pybella/main.py:7
- [nitpick] The
prepareimport is unused in this module; consider removing it to clean up imports.
from .flow_solver.utils import prepare
Owner
Author
This was referenced Jun 16, 2025
Closed
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.

Intermediate merge before the last push for the implicit solver.