Add conductivity boundaries to Dirichlet BC list for wave ports #264
Conversation
sebastiangrimberg
added
usage
sebastiangrimberg
force-pushed
the
sjg/wave-port-bcs
branch
from
6fb1bb3
to
e3c32de
Compare
…alculation Also removes old FEAST code from docs, simplifies ability to specify eigenmode solver to wave ports.
| ARPACK, | ||
| FEAST | ||
| }; | ||
| using Type = WavePortData::EigenSolverType; |
There was a problem hiding this comment.
Seems a little surprising to have the EigenSolverType a subtype within WavePortData rather than freestanding?
There was a problem hiding this comment.
I agree I didn't love this but also wanted to be consistent that everywhere else enums are defined in the scope of a struct which uses them. The fact that it is defined here instead of in EigenSolverData as it was previously is just because of ordering in the file. Is there a better solution?
There was a problem hiding this comment.
Practically it doesn't make that much of a difference ultimately given this type alias, so not a big deal. It does highlight the difficulty of that convention though, as it introduces this side coupling. I would prefer the enum outside, as at this point the notion of an EigenSolverType is "more common", but it's just a style thing so consistency is king here.
There was a problem hiding this comment.
Yes I would support the refactor to having all enums be described outside the structs, and we could us using statements in the struct scope to avoid needing naming changes to access them. That is a good idea for a later PR to make it all consistent.
| mu_eps_min = 1.0 / (c_max * c_max) * 0.5; // Add a safety factor for minimum propagation | ||
| // constant possible |
There was a problem hiding this comment.
What's the origin of this safety factor? Is it a floating point issue?
There was a problem hiding this comment.
Yeah, if the wave port is specified on a homogeneous waveguide supporting a TEM mode with no cutoff, then setting the safety factor of 1 will lead to the shift for the shift-and-invert transformation being directly on top of the actual lowest eigenvalue which is going to cause problems. Setting the shift to 0 works fine (and was what we did previously) but having some non-zero shift can speed up the eigenvalue solve convergence. It didn't make a significant difference in my testing but I figured it might as well be some positive value.
| @@ -138,21 +138,9 @@ number of eigenmodes of the problem. The available options are: | |||
|
|
|||
| - `"SLEPc"` | |||
| - `"ARPACK"` | |||
| - `"FEAST"` | |||
There was a problem hiding this comment.
It can come back when needed! Commit c435ace has the original code which just needs minor modifications after the linear algebra refactors of last year.
| ARPACK, | ||
| FEAST | ||
| }; | ||
| using Type = WavePortData::EigenSolverType; |
There was a problem hiding this comment.
Practically it doesn't make that much of a difference ultimately given this type alias, so not a big deal. It does highlight the difficulty of that convention though, as it introduces this side coupling. I would prefer the enum outside, as at this point the notion of an EigenSolverType is "more common", but it's just a style thing so consistency is king here.
Wave port mode calculation right now inherits all PEC boundary conditions from the 3D problem. Conductivity boundaries should also be considered as PEC for mode calculation (otherwise it was expected the user adds them with
"WavePortPEC").Also updates the eigenvalue target for the shifted wave port eigemode problem to
EigenvalueSolver::WhichType::SMALLEST_REAL, which appropriately deprioritizes evanescent modes with large imaginary part and small real part ofk_n.