Add support for uniform grid sizing across subdomains (#720) by swastim01 · Pull Request #5253 · pybamm-team/PyBaMM

swastim01 · 2025-10-30T08:05:54Z

Description

This PR adds functionality to enable uniform grid sizing across subdomains in the x-dimension, addressing issue #720.

Previously, grid points had to be specified manually for each subdomain, which could result in non-uniform spacing across domain interfaces.
With this update, the user can now define grid points in one subdomain, and the remaining subdomains automatically adopt grid sizes that ensure consistent spacing across the full geometry.

Key changes:

Added logic in meshes.py to automatically compute grid points for adjacent subdomains based on the uniform dx.
Added input validation to ensure minimum grid size (at least two points per subdomain).
Aligned implementation with existing mesh generation workflow for full backward compatibility.

This feature simplifies geometry setup and ensures smooth mesh transitions between subdomains for both 1D and higher-dimensional cases.

Fixes #720

Type of change

New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Refactor (code change that improves readability/structure without changing functionality)
Documentation update

Important checks:

Please confirm the following before marking the PR as ready for review:

No style issues: nox -s pre-commit
All tests pass: nox -s tests
The documentation builds: nox -s doctests
Code is commented for hard-to-understand areas
Tests added that prove fix is effective or that feature works

Open to feedback/suggestions. I was wondering if i should add a dedicated test (e.g., test_uniform_grid_sizing) in test_meshes.py, or prefer a new test file under tests/unit/test_meshes/?

codecov · 2025-10-30T14:59:18Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.77%. Comparing base (df2dbbd) to head (3faf329).

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #5253   +/-   ##
========================================
  Coverage    98.77%   98.77%           
========================================
  Files          321      321           
  Lines        27717    27727   +10     
========================================
+ Hits         27377    27387   +10     
  Misses         340      340

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

valentinsulzer

Thanks for taking this on!
I think the current implementation is a little bit arbitrary - since it's hard to know which of the variables will be "found" first to be set as the sepcified domain.
Instead, I would recommend having a helper function:

input: electrode thicknesses, grid size required
output: var pts dict
PS feel free to ping me on slack for a re-review, I don't always check pybamm PR review requests

… grid size Includes a unit test validating proportional scaling across domains.

valentinsulzer

thanks, this is cleaner, but could we make grid_size the size of the mesh cell in metres (dx in your code) so it's very explicit

…ts_from_thicknesses, edited added test accordingly

valentinsulzer

looks great, thanks!

agriyakhetarpal · 2025-11-15T02:02:08Z

@swastim01, it looks like the only thing remaining for this PR is for you to fix the two lines of missing coverage. It looks trivial to me. Would you like to try it?

swastim01 · 2025-11-15T09:57:54Z

Thanks for pointing that out. I’ve added the extra tests to cover the uncovered lines, so the patch should now have full coverage. Happy to make any further adjustments if needed.

agriyakhetarpal

Thanks, looks good, @swastim01! Let's merge this once tests pass.

Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com>

* Don't be too strict with func_args longer than symbol.children * Add a test * Add support for uniform grid sizing across subdomains (#720) (#5253) Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com> * Fix typo in Butler-Volmer equation docstring (#5279) * fix bug with bulk ocp lithiation (#5280) * doc: fix typo in concentration description in notebook (#5284) * Fix typo in concentration description in notebook * Add CHANGELOG.md entry for typo fix * Remove unneccesary changelog entry Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> --------- Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> * fix: instruct uv to install into system for CI (#5288) * Fix `InputParameter` serialisation (#5289) * fix `InputParameter` serialisation * Update CHANGELOG.md * Bugfix: inputs for `initial_conditions_from` scale evaluation (#5285) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Add `silence_sundials_errors` solver option (#5290) * feat: add`silence_sundial_warnings` solver option * refactor: `silence_sundials_warnings` -> `silence_sundials_errors` * Update C-Rate current for changing nominal capacity (#5286) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Update `IDAKLUSolver` error handling (#5291) * raise `SolverError` at failure to init sundials * Update simulation.py * Update idaklu_solver.py * reuse `pybammsolvers` error messages * Update test_idaklu_solver.py * bump `pybammsolvers` * Update CHANGELOG.md * Update CHANGELOG.md Update CHANGELOG.md * #5299 add inverse linear kinetics * #5299 check domain options for inverse kinetics * update notebook * #5299 brady comments * #5299 fix composite total kinetics --------- Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Co-authored-by: Swasti Mishra <140950062+swastim01@users.noreply.github.com> Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com> Co-authored-by: Chase Naples <cnaples79@gmail.com> Co-authored-by: Gregor Decristoforo <gregor.decristoforo@gmail.com> Co-authored-by: Pip Liggins <philippa.liggins@dtc.ox.ac.uk> Co-authored-by: Marc Berliner <34451391+MarcBerliner@users.noreply.github.com> Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

…#5308) * Don't be too strict with func_args longer than symbol.children * Add a test * Add support for uniform grid sizing across subdomains (#720) (#5253) Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com> * Fix typo in Butler-Volmer equation docstring (#5279) * fix bug with bulk ocp lithiation (#5280) * doc: fix typo in concentration description in notebook (#5284) * Fix typo in concentration description in notebook * Add CHANGELOG.md entry for typo fix * Remove unneccesary changelog entry Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> --------- Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> * fix: instruct uv to install into system for CI (#5288) * Fix `InputParameter` serialisation (#5289) * fix `InputParameter` serialisation * Update CHANGELOG.md * Bugfix: inputs for `initial_conditions_from` scale evaluation (#5285) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Add `silence_sundials_errors` solver option (#5290) * feat: add`silence_sundial_warnings` solver option * refactor: `silence_sundials_warnings` -> `silence_sundials_errors` * Update C-Rate current for changing nominal capacity (#5286) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Update `IDAKLUSolver` error handling (#5291) * raise `SolverError` at failure to init sundials * Update simulation.py * Update idaklu_solver.py * reuse `pybammsolvers` error messages * Update test_idaklu_solver.py * bump `pybammsolvers` * Update CHANGELOG.md * Update CHANGELOG.md Update CHANGELOG.md * revive `SymbolReplacer` * add `VariableReplacementMap` * Create replace_symbols.rst * Update index.rst * cleanup * attach `parameter_values` to model for lazy post-processing * update `model.parameter_values` serialization * Update test_serialisation.py * Update test_parameter_values.py * add `observe` to solution for lazy evaluation of discretized symbols * Update test_solution.py * style * Update CHANGELOG.md * fix docstrings * docs * docs2 * Update replace_symbols.py * Update replace_symbols.py * remove `SymbolReplacer` * add slots to `SpatialMethod` for memory * add `SymbolProcessor` for `BaseModel` symbol eval * add `ModelSolutionObservability` enums for `BaseModel` * parameter_values: delay variable processing * fix stale symbol id: use `scale` and `reference` setters * discretisation delayed var processing, don't overwrite `model.variables` * pass `delayed_variable_processing` args through `process_model` * add `DUMMY_INPUT_PARAMETER_VALUE` const * Update __init__.py * Update index.rst * Delete replace_symbols.py * update symbol `scale` and `reference` setters * update variable hash * explicit `_variables_processed` base model attribute * use `get_processed_variable_or_event` getter * copy model in `Simulation` * disable observability with `output_variables` * delay variable processing in `Simulation` build * add `Solution.observe(symbol)` * update serialization * update examples * Update CONTRIBUTING.md * Update CHANGELOG.md * add observe tests * update serialization tests * Update test_base_model.py * update symbol processor getters * add model observability tests * update symbol and base variable id setters * Update symbol_processor.py * Update CHANGELOG.md * fix model serialization notebook * Update symbol_processor.py * remove unused serialisation methods * remove debug statement * update observe test comments * add observe test failure for new parameters * remove `_solution_observable` from custom model serialization * fix observe test error * set `DUMMY_INPUT_PARAMETER_VALUE = np.nan` to make errors more obvious --------- Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Co-authored-by: Swasti Mishra <140950062+swastim01@users.noreply.github.com> Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com> Co-authored-by: Chase Naples <cnaples79@gmail.com> Co-authored-by: Robert Timms <43040151+rtimms@users.noreply.github.com> Co-authored-by: Gregor Decristoforo <gregor.decristoforo@gmail.com> Co-authored-by: Pip Liggins <philippa.liggins@dtc.ox.ac.uk> Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Add error * Add test * Add to changelog, edit message * Don't be too strict with func_args longer than symbol.children * Add a test * Add support for uniform grid sizing across subdomains (#720) (#5253) Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com> * Fix typo in Butler-Volmer equation docstring (#5279) * fix bug with bulk ocp lithiation (#5280) * doc: fix typo in concentration description in notebook (#5284) * Fix typo in concentration description in notebook * Add CHANGELOG.md entry for typo fix * Remove unneccesary changelog entry Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> --------- Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> * fix: instruct uv to install into system for CI (#5288) * Fix `InputParameter` serialisation (#5289) * fix `InputParameter` serialisation * Update CHANGELOG.md * Bugfix: inputs for `initial_conditions_from` scale evaluation (#5285) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --------- Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Co-authored-by: Swasti Mishra <140950062+swastim01@users.noreply.github.com> Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com> Co-authored-by: Chase Naples <cnaples79@gmail.com> Co-authored-by: Robert Timms <43040151+rtimms@users.noreply.github.com> Co-authored-by: Gregor Decristoforo <gregor.decristoforo@gmail.com> Co-authored-by: Marc Berliner <34451391+MarcBerliner@users.noreply.github.com> Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Martin Robinson <martinjrobins@gmail.com>

* Add error * Add test * Add to changelog, edit message * Don't be too strict with func_args longer than symbol.children * Add a test * Add support for uniform grid sizing across subdomains (pybamm-team#720) (pybamm-team#5253) Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com> * Fix typo in Butler-Volmer equation docstring (pybamm-team#5279) * fix bug with bulk ocp lithiation (pybamm-team#5280) * doc: fix typo in concentration description in notebook (pybamm-team#5284) * Fix typo in concentration description in notebook * Add CHANGELOG.md entry for typo fix * Remove unneccesary changelog entry Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> --------- Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> * fix: instruct uv to install into system for CI (pybamm-team#5288) * Fix `InputParameter` serialisation (pybamm-team#5289) * fix `InputParameter` serialisation * Update CHANGELOG.md * Bugfix: inputs for `initial_conditions_from` scale evaluation (pybamm-team#5285) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --------- Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Co-authored-by: Swasti Mishra <140950062+swastim01@users.noreply.github.com> Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com> Co-authored-by: Chase Naples <cnaples79@gmail.com> Co-authored-by: Robert Timms <43040151+rtimms@users.noreply.github.com> Co-authored-by: Gregor Decristoforo <gregor.decristoforo@gmail.com> Co-authored-by: Marc Berliner <34451391+MarcBerliner@users.noreply.github.com> Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Martin Robinson <martinjrobins@gmail.com>

* Don't be too strict with func_args longer than symbol.children * Add a test * Add support for uniform grid sizing across subdomains (#720) (#5253) Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com> * Fix typo in Butler-Volmer equation docstring (#5279) * fix bug with bulk ocp lithiation (#5280) * doc: fix typo in concentration description in notebook (#5284) * Fix typo in concentration description in notebook * Add CHANGELOG.md entry for typo fix * Remove unneccesary changelog entry Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> --------- Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> * fix: instruct uv to install into system for CI (#5288) * Fix `InputParameter` serialisation (#5289) * fix `InputParameter` serialisation * Update CHANGELOG.md * Bugfix: inputs for `initial_conditions_from` scale evaluation (#5285) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Add `silence_sundials_errors` solver option (#5290) * feat: add`silence_sundial_warnings` solver option * refactor: `silence_sundials_warnings` -> `silence_sundials_errors` * Update C-Rate current for changing nominal capacity (#5286) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Update `IDAKLUSolver` error handling (#5291) * raise `SolverError` at failure to init sundials * Update simulation.py * Update idaklu_solver.py * reuse `pybammsolvers` error messages * Update test_idaklu_solver.py * bump `pybammsolvers` * Update CHANGELOG.md * Update CHANGELOG.md Update CHANGELOG.md * implement heat equation export * Move DiffslExport to evaluate_python.py Co-authored-by: martinjrobins <martinjrobins@gmail.com> * add tests for DiffSLExport * rename DiffslExport to DiffSLExport * consider both options for heat flux output * move diffsl code to new file * add spm, spme and dfn tests, apply fixes for these * can export dfn and spme, but output is incorrect * new input syntax * move model export to integration, tidy up float formattting * update changelog * update to new delayed variable pipeline * minor fixes, remove dead code * turn off pydiffsol dep for mac intel * don't use inplace for integration tests * improve coverage * fix: reg functions * doc: minor comment to explain function fallback * refactor: eqn -> eqn_str * refactor: use ordered dict, remove unnecessary code * test: remove diffsl integration tests * test: add snapshot tests for unit diffsl export tests * test: reg_power and arcsinh2 * dep: remove pydiffsol --------- Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Co-authored-by: Swasti Mishra <140950062+swastim01@users.noreply.github.com> Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com> Co-authored-by: Chase Naples <cnaples79@gmail.com> Co-authored-by: Robert Timms <43040151+rtimms@users.noreply.github.com> Co-authored-by: Gregor Decristoforo <gregor.decristoforo@gmail.com> Co-authored-by: Pip Liggins <philippa.liggins@dtc.ox.ac.uk> Co-authored-by: Marc Berliner <34451391+MarcBerliner@users.noreply.github.com> Co-authored-by: Brady Planden <55357039+BradyPlanden@users.noreply.github.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

swastim01 requested a review from a team as a code owner October 30, 2025 08:05

Add support for uniform grid sizing across subdomains (pybamm-team#720)

5567fc0

swastim01 force-pushed the uniform-grid-sizing branch from 159fbc8 to 5567fc0 Compare October 30, 2025 08:25

docs(changelog): add entry for uniform grid sizing across subdomains

14e67ae

agriyakhetarpal requested a review from valentinsulzer October 30, 2025 14:39

swastim01 and others added 2 commits October 30, 2025 21:45

Merge branch 'pybamm-team:develop' into uniform-grid-sizing

da3f905

Merge branch 'develop' into uniform-grid-sizing

bb6c69b

valentinsulzer requested changes Oct 31, 2025

View reviewed changes

Add helper function to compute var_pts from electrode thicknesses and…

58eb596

… grid size Includes a unit test validating proportional scaling across domains.

valentinsulzer requested changes Oct 31, 2025

View reviewed changes

swastim01 added 2 commits October 31, 2025 22:10

Refactor: interpret grid_size as mesh cell size (dx) in compute_var_p…

3ee0c45

…ts_from_thicknesses, edited added test accordingly

Fix: revert unintended Mesh constructor change

e7a2680

valentinsulzer previously approved these changes Oct 31, 2025

View reviewed changes

added missing coverage

72607df

swastim01 dismissed valentinsulzer’s stale review via 72607df November 15, 2025 09:55

Merge branch 'develop' into uniform-grid-sizing

3faf329

agriyakhetarpal approved these changes Nov 15, 2025

View reviewed changes

agriyakhetarpal merged commit 425af11 into pybamm-team:develop Nov 15, 2025
37 of 38 checks passed

swastim01 deleted the uniform-grid-sizing branch November 16, 2025 04:40

BradyPlanden pushed a commit that referenced this pull request Dec 8, 2025

Add support for uniform grid sizing across subdomains (#720) (#5253)

0c92f94

Co-authored-by: Valentin Sulzer <valentinsulzer@hotmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add support for uniform grid sizing across subdomains (#720)#5253

Add support for uniform grid sizing across subdomains (#720)#5253
agriyakhetarpal merged 9 commits intopybamm-team:developfrom
swastim01:uniform-grid-sizing

swastim01 commented Oct 30, 2025 •

edited

Loading

Uh oh!

codecov Bot commented Oct 30, 2025 •

edited

Loading

Uh oh!

valentinsulzer left a comment

Uh oh!

valentinsulzer left a comment

Uh oh!

valentinsulzer left a comment

Uh oh!

agriyakhetarpal commented Nov 15, 2025

Uh oh!

swastim01 commented Nov 15, 2025

Uh oh!

agriyakhetarpal left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

swastim01 commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Important checks:

Uh oh!

codecov Bot commented Oct 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

valentinsulzer left a comment

Choose a reason for hiding this comment

Uh oh!

valentinsulzer left a comment

Choose a reason for hiding this comment

Uh oh!

valentinsulzer left a comment

Choose a reason for hiding this comment

Uh oh!

agriyakhetarpal commented Nov 15, 2025

Uh oh!

swastim01 commented Nov 15, 2025

Uh oh!

agriyakhetarpal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

swastim01 commented Oct 30, 2025 •

edited

Loading

codecov Bot commented Oct 30, 2025 •

edited

Loading