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

Expose inter-/histopolation matrices in GlobalProjector, add unit tests #347

Merged
merged 31 commits into from
Feb 12, 2025

Conversation

spossann
Copy link
Collaborator

@spossann spossann commented Oct 12, 2023

Fixes #346.

The inter-/histopolation matrices are now exposed in GlobalProjector.imat_kronecker as a LinearOperator object of type KroneckerStencilMatrix (in the case where both domain and codomain are scalar spaces) or BlockLinearOperator (in the case where at least one space is vector valued).

The unit tests in feec/tests/test_commuting_projections.py have been enhanced: they now also test how close imat_kronecker is to being the right-inverse of the solverattribute (which uses a Kronecker product of 1D linear solvers).

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@spossann spossann requested a review from yguclu October 12, 2023 13:42
@spossann
Copy link
Collaborator Author

Hi @yguclu, it seems I cannot see why the latest test failed, do you have an idea? Also, did you have time to think about this PR yet?

psydac/feec/tests/test_commuting_projections.py Outdated Show resolved Hide resolved
psydac/feec/tests/test_commuting_projections.py Outdated Show resolved Hide resolved
@spossann
Copy link
Collaborator Author

Python 3.11 tests passed upon relaunching test, without changes.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@yguclu
Copy link
Member

yguclu commented Dec 5, 2023

@spossann It seems that the code could be simplified with these steps:

  1. Use the function array_to_psydac in module psydac.linalg.utilities to convert the 1D matrices from Numpy arrays to a StencilMatrix format (not distributed);
  2. Create a distributed 3D KroneckerStencilMatrix object from the serial 1D StencilMatrix objects;
  3. If needed, call the method KroneckerStencilMatrix.tostencil() to obtain a distributed 3D StencilMatrix object.

Before making any of these changes, could you please add a parallel test to this PR?

@spossann
Copy link
Collaborator Author

@spossann It seems that the code could be simplified with these steps:

1. Use the function `array_to_psydac` in module `psydac.linalg.utilities` to convert the 1D matrices from Numpy arrays to a `StencilMatrix` format (not distributed);

The function array_to_psydac converts numpy arrays into Stencil/BlockVector only (not Matrix).

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
spossann and others added 5 commits September 13, 2024 14:48

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Co-authored-by: Yaman Güçlü <[email protected]>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Change `IdentityLinearOperator` to `IdentityOperator`, and `e0 @ e0` to
`e0.dot(e0)`. Add missing import statement.
…1, this fixes the problem with toarray() which did previously not work for StencilMatrix. Updated indexing in KroneckerStencilMatrix to include shift factors in index calculations. Added back the assert statement in GlobalProjector since the toarray() now works. The kernels still don't work properly for different shifts in the domain and the co-domain, but I believe equivalent changes can be done in the remaining kernels.
yguclu and others added 3 commits October 1, 2024 15:13
- Add compiler flags for GFortran >= 14 on Apple silicon chips
- Add unit test in `test_epyccel_flags.py` to check compiler flags
passed to `epyccel`
- Update GitHub actions used in documentation workflow
NumPy 2.1 deprecates the `newshape` argument of the `reshape` function.
Instead, one should use the positional-only argument `shape`. Every call
in Psydac has been updated accordingly. This closes #432.
Update `apt` index files with `apt-get update` to avoid download error
in CI workflow.
yguclu and others added 6 commits October 1, 2024 15:13
Clarify in the `README.md` file that, although the C backend may be
selected for accelerating the kernel files with Pyccel, this is not
fully working yet. Hence the Fortran backend (which is the default) is
the only one available. A future version of Pyccel will certainly
provide a C backend as capable as the Fortran one. See issue #431.

Co-authored-by: Martin Campos Pinto <[email protected]>
The obsolete property `is_block` was never used in Psydac nor in
Struphy, hence we remove it.

---------

Co-authored-by: Yaman Güçlü <[email protected]>
Fix the indexing in all kernels in `stencil2coo_kernels.py` so that the
method `toarray` works for `StencilMatrix` objects in 1D/2D/3D with
`shift` larger than 1.

The unit tests for `toarray` in `test_stencil_matrix.py` have been
parametrized to run with `shift` values of 1 and 2.

---------

Co-authored-by: Yaman Güçlü <[email protected]>
---------

Co-authored-by: Yaman Güçlü <[email protected]>
…lation-matrices
psydac/feec/tests/test_commuting_projections.py Outdated Show resolved Hide resolved
psydac/feec/tests/test_commuting_projections.py Outdated Show resolved Hide resolved
psydac/feec/tests/test_commuting_projections.py Outdated Show resolved Hide resolved
psydac/feec/tests/test_commuting_projections.py Outdated Show resolved Hide resolved
psydac/feec/tests/test_commuting_projections.py Outdated Show resolved Hide resolved
psydac/feec/tests/test_commuting_projections.py Outdated Show resolved Hide resolved
psydac/feec/tests/test_commuting_projections.py Outdated Show resolved Hide resolved
@max-models
Copy link
Contributor

Pushed with the 3D tests uncommented to ensure they all go through.

psydac/feec/tests/test_commuting_projections.py Outdated Show resolved Hide resolved
psydac/feec/tests/test_commuting_projections.py Outdated Show resolved Hide resolved
yguclu
yguclu previously requested changes Oct 23, 2024
.github/workflows/continuous-integration.yml Outdated Show resolved Hide resolved
@max-models max-models requested a review from yguclu November 2, 2024 08:23
@yguclu yguclu added the urgent PR should be merged ASAP label Nov 13, 2024
Copy link

codacy-production bot commented Feb 5, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.07% 100.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (66205a5) 30663 18399 60.00%
Head commit (230eba4) 61406 (+30743) 36886 (+18487) 60.07% (+0.07%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#347) 46 46 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

@max-models max-models dismissed yguclu’s stale review February 6, 2025 09:36

All requests have already been addressed.

Copy link
Member

@yguclu yguclu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done! This can be merged finally 🥳

@yguclu yguclu merged commit 5669252 into devel Feb 12, 2025
9 checks passed
@yguclu yguclu deleted the expose-inter-histopolation-matrices branch February 12, 2025 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
urgent PR should be merged ASAP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose inter-/histopolation matrices as BlockLinearOperator in GlobalProjector
5 participants