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

Complex arithmetic for PDE solvers 2 #755

Merged
merged 18 commits into from Oct 18, 2019
Merged

Complex arithmetic for PDE solvers 2 #755

merged 18 commits into from Oct 18, 2019

Conversation

sdrave
Copy link
Member

@sdrave sdrave commented Aug 26, 2019

This PR adds generic support for complex-valued VectorArrays for PDE solvers with ListVectorArray-based bindings. The construction is generic and does not require any complex number support within the PDE solver. The approach is similar to #747, but provides base classes for Vectors and Operators that can be used for arbitrary solver bindings.

Unlike #747, complex linear combinations are completely handled by LincombOperator. Should the need arise, an assemble_lincomb rule could be added, which separately assembles real and imaginary parts.

Moreover, this PR adds complex data to the VectorArray fixtures and fixes various complex number issues, both in the tests and the implementations.

I have tried to fix lgmres, but more testing / checking the code is required. The heat_sink notebook seems to work, although the error system magnitude plot looks different. (At first sight, both plots look strange, and maybe the lgmres tolerances are not chosen correctly.) lgmres seems to run significantly faster compared to #747.

@sdrave sdrave added pr:new-feature bindings labels Aug 26, 2019
@sdrave sdrave added this to the 2019.2 milestone Aug 26, 2019
@sdrave sdrave requested review from renefritze and pmli Aug 26, 2019
@codecov
Copy link

@codecov codecov bot commented Aug 26, 2019

Codecov Report

Merging #755 into master will increase coverage by 0.05%.
The diff coverage is 74.48%.

Impacted Files Coverage Δ
src/pymor/algorithms/genericsolvers.py 78.26% <100%> (+0.13%) ⬆️
src/pymor/operators/basic.py 60.47% <53.24%> (-3.17%) ⬇️
src/pymor/bindings/fenics.py 72.82% <62.71%> (-0.34%) ⬇️
src/pymor/bindings/ngsolve.py 84.84% <84.78%> (+1%) ⬆️
src/pymor/vectorarrays/list.py 84.9% <85.8%> (-0.14%) ⬇️
src/pymor/reductors/basic.py 77.58% <0%> (ø) ⬆️
src/pymor/vectorarrays/numpy.py 84.05% <0%> (+0.25%) ⬆️
src/pymor/discretizers/cg.py 91.66% <0%> (+1.34%) ⬆️
src/pymor/models/interfaces.py 82.35% <0%> (+2.35%) ⬆️
... and 1 more

2 similar comments
@codecov
Copy link

@codecov codecov bot commented Aug 26, 2019

Codecov Report

Merging #755 into master will increase coverage by 0.05%.
The diff coverage is 74.48%.

Impacted Files Coverage Δ
src/pymor/algorithms/genericsolvers.py 78.26% <100%> (+0.13%) ⬆️
src/pymor/operators/basic.py 60.47% <53.24%> (-3.17%) ⬇️
src/pymor/bindings/fenics.py 72.82% <62.71%> (-0.34%) ⬇️
src/pymor/bindings/ngsolve.py 84.84% <84.78%> (+1%) ⬆️
src/pymor/vectorarrays/list.py 84.9% <85.8%> (-0.14%) ⬇️
src/pymor/reductors/basic.py 77.58% <0%> (ø) ⬆️
src/pymor/vectorarrays/numpy.py 84.05% <0%> (+0.25%) ⬆️
src/pymor/discretizers/cg.py 91.66% <0%> (+1.34%) ⬆️
src/pymor/models/interfaces.py 82.35% <0%> (+2.35%) ⬆️
... and 1 more

@codecov
Copy link

@codecov codecov bot commented Aug 26, 2019

Codecov Report

Merging #755 into master will increase coverage by 0.05%.
The diff coverage is 74.48%.

Impacted Files Coverage Δ
src/pymor/algorithms/genericsolvers.py 78.26% <100%> (+0.13%) ⬆️
src/pymor/operators/basic.py 60.47% <53.24%> (-3.17%) ⬇️
src/pymor/bindings/fenics.py 72.82% <62.71%> (-0.34%) ⬇️
src/pymor/bindings/ngsolve.py 84.84% <84.78%> (+1%) ⬆️
src/pymor/vectorarrays/list.py 84.9% <85.8%> (-0.14%) ⬇️
src/pymor/reductors/basic.py 77.58% <0%> (ø) ⬆️
src/pymor/vectorarrays/numpy.py 84.05% <0%> (+0.25%) ⬆️
src/pymor/discretizers/cg.py 91.66% <0%> (+1.34%) ⬆️
src/pymor/models/interfaces.py 82.35% <0%> (+2.35%) ⬆️
... and 1 more

@codecov
Copy link

@codecov codecov bot commented Aug 26, 2019

Codecov Report

Merging #755 into master will increase coverage by 0.05%.
The diff coverage is 74.84%.

Impacted Files Coverage Δ
src/pymor/algorithms/genericsolvers.py 78.26% <100%> (+0.13%) ⬆️
src/pymor/operators/basic.py 60.47% <53.24%> (-3.17%) ⬇️
src/pymor/bindings/fenics.py 72.82% <62%> (-0.34%) ⬇️
src/pymor/vectorarrays/list.py 84.9% <85.8%> (-0.14%) ⬇️
src/pymor/bindings/ngsolve.py 84.84% <86.36%> (+1%) ⬆️
src/pymor/reductors/basic.py 77.58% <0%> (ø) ⬆️
src/pymor/vectorarrays/numpy.py 84.05% <0%> (+0.25%) ⬆️
src/pymor/discretizers/cg.py 91.66% <0%> (+1.34%) ⬆️
... and 2 more

Copy link
Member

@renefritze renefritze left a comment

How would this work if I got a Fenics install that supports complex field types natively?

@sdrave
Copy link
Member Author

@sdrave sdrave commented Aug 27, 2019

How would this work if I got a Fenics install that supports complex field types natively?

Not sure if I understand the question. If FEniCS would support complex numbers, this feature would be ignored by the bindings as only real numbers will be passed to the backend. If one wants to use native complex number support one would have to adapt the bindings.

@renefritze
Copy link
Member

@renefritze renefritze commented Aug 29, 2019

Since DolfinX does support complex natively, I think we should at least consider this when implementing these changes to not make future support for that unnecessarily hard.

@pmli
Copy link
Member

@pmli pmli commented Aug 29, 2019

Since DolfinX does support complex natively, I think we should at least consider this when implementing these changes to not make future support for that unnecessarily hard.

I would agree. Is the idea to add a FenicsXVector at some point or have a real and complex version of FenicsVector as for NGSolve?

@sdrave
Copy link
Member Author

@sdrave sdrave commented Aug 30, 2019

I don't know how native complex data will be supported by the FEniCS bindings, as I know nothing about complex number support in DolfinX. I assume that most code can be shared, but there will probably be two versions of the bindings.

Is the idea to add a FenicsXVector at some point or have a real and complex version of FenicsVector as for NGSolve?

I am not sure if I understand the difference. For NGSolve it is possible to instantiate complex discrete function spaces. I suspect, that then also all matrices assembled for this function space will have complex data. I don't know (haven't tried yet) if it is possible to multiply a real matrix with a complex vector. If yes, it would be possible, to dispose the complexified NGSolve bindings from this PR and only use the native support. I would do the same thing for DolfinX. If the backend cannot handle a mix of real an complex data, it will be probably necessary to keep the complexified bindings for the case when the actual model is real but we need to solve problems with complex shifts and offer a second set of bindings for the case when the actual model is using complex numbers.

If you have anything concrete in mind we can do know to simplify things in the future, I am open to suggestions ..

src/pymor/vectorarrays/list.py Outdated Show resolved Hide resolved
@sdrave
Copy link
Member Author

@sdrave sdrave commented Oct 18, 2019

@pml, can this be merged?

@pmli
Copy link
Member

@pmli pmli commented Oct 18, 2019

Yes.

@sdrave sdrave merged commit c6ba537 into master Oct 18, 2019
7 checks passed
@sdrave sdrave deleted the complex-op-va2 branch Oct 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:new-feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants