Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unify refine interface #3322

Merged
merged 10 commits into from
Sep 16, 2024
Merged

Conversation

schnellerhase
Copy link
Contributor

@schnellerhase schnellerhase commented Jul 24, 2024

This PR contains several changes to the refinement routines to facilitate a neater, simpler and combined API to the user.

  1. The interface of the plaza routines now uses std::optional's as input arguments and thus allows for a single code path. (Previously the cases of uniform and adaptive refinement were treated independently)
  2. The plaza refinement options was centralized to the (more general) refinement namespace and now is also used for configuring the interval refinement.
  3. The return values of parent facets and cells are now std::optional as well, to no longer need to rely on an ambiguous empty array check.
  4. The python export and interface with the new optional arguments is greatly simplified, no longer do we need to export multiple versions of the refinement routines, and no more dispatching in the python interface is necessary.
  5. Refinement testing has been extended and simplified in some cases.

@schnellerhase
Copy link
Contributor Author

schnellerhase commented Jul 24, 2024

  • Add functionality of retrieving the parent facet/vertex indices with the interval refinement
  • Move refinement::plaza:Option for controlling what gets computed to central location
  • Make ghost mode option of plaza
  • Make interval refinement available through refine()
  • python exports simplifications, one function one export - optional arguments should be treated implicitly by python caller
  • Should probably return a shared_ptr to the new mesh, for consistent interface usage with the external API

@schnellerhase schnellerhase force-pushed the unify_refine_interface branch 6 times, most recently from 59a6661 to 02e7fd3 Compare July 30, 2024 16:34
@schnellerhase schnellerhase marked this pull request as ready for review July 30, 2024 16:44
@jhale
Copy link
Member

jhale commented Aug 1, 2024

"Should probably return a shared_ptr to the new mesh, for consistent interface usage with the external API" - This isn't necessary, as the caller takes ownership.

Could you try std::optional on the return types from refine? This would avoid using empty arrays as sentinels for "not computed".

@schnellerhase
Copy link
Contributor Author

Optional return value seems to work as well, in python this just becomes a None value if nothing is set :) - will change the facets as well to this behavior.

@schnellerhase schnellerhase force-pushed the unify_refine_interface branch 2 times, most recently from e269508 to 5bdc17e Compare August 5, 2024 11:00
@schnellerhase
Copy link
Contributor Author

schnellerhase commented Aug 5, 2024

Requires prior merge of #3331

@mscroggs mscroggs added this to the 0.10.0 milestone Sep 3, 2024
@garth-wells
Copy link
Member

@schnellerhase could you resolve the merge conflict?

@garth-wells garth-wells added this pull request to the merge queue Sep 16, 2024
Merged via the queue into FEniCS:main with commit 1bff1d4 Sep 16, 2024
15 checks passed
@schnellerhase schnellerhase deleted the unify_refine_interface branch September 19, 2024 11:38
francesco-ballarin added a commit to FEniCS/performance-test that referenced this pull request Nov 3, 2024
github-merge-queue bot pushed a commit to FEniCS/performance-test that referenced this pull request Nov 3, 2024
* Pin versions in CI workflow for release 0.9.0

* Add files generate by cmake to .gitignore

* Bring dolfinx::refinement::refine up to date with dolfinx 0.9.0

Due to changes in
FEniCS/dolfinx#3322
FEniCS/dolfinx#3444
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants