Necessary changes required to be able to instantiate a TrialFESpace with an instance of DistributedCellField #185
Conversation
with an instance of DistributedCellField
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for creating TrialFESpace with DistributedCellField objects as boundary/initial data. The key changes include:
- Added two new
TrialFESpaceconstructor overloads that acceptDistributedCellFieldarguments - Added a new
interpolate_dirichlet!method to handleDistributedCellFieldinterpolation - Refactored test code to verify interpolation works with both Julia functions and
DistributedCellFieldobjects
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/FESpaces.jl | Added new overloads for TrialFESpace and interpolate_dirichlet! to support DistributedCellField arguments |
| test/FESpacesTests.jl | Refactored interpolation tests into a helper function and added tests for DistributedCellField interpolation |
| NEWS.md | Documented the new feature in the changelog |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function FESpaces.TrialFESpace(cf::DistributedCellField,f::DistributedSingleFieldFESpace) | ||
| spaces = map(local_views(f),local_views(cf)) do s, field | ||
| TrialFESpace(s,field) | ||
| end | ||
| DistributedSingleFieldFESpace(spaces,f.gids,f.trian,f.vector_type,f.metadata) | ||
| end | ||
|
|
||
|
|
There was a problem hiding this comment.
[nitpick] The two TrialFESpace overloads at lines 417-422 and 424-429 have identical implementations. The second overload (where DistributedCellField is the first argument) simply calls the same underlying Gridap TrialFESpace(s,field) function. Consider consolidating these into a single implementation or adding a comment explaining why both argument orders are necessary.
| function FESpaces.TrialFESpace(cf::DistributedCellField,f::DistributedSingleFieldFESpace) | |
| spaces = map(local_views(f),local_views(cf)) do s, field | |
| TrialFESpace(s,field) | |
| end | |
| DistributedSingleFieldFESpace(spaces,f.gids,f.trian,f.vector_type,f.metadata) | |
| end |
| uh4 = FEFunction(U,free_values,dirichlet_values) | ||
| eh4 = u - uh4 | ||
|
|
||
| dΩ = Measure(Ω,3) |
There was a problem hiding this comment.
The variable dΩ is redefined inside _interpolation_tests at line 131, shadowing the outer dΩ defined at line 88. While this works, it's confusing and the outer dΩ at line 88 appears to be unused. Consider removing the outer definition or using a different variable name in the inner function.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Do not merge yet.
Requires Gridap's PR 1117 to be released.
Related to issue #184