jorgensd / dolfinx_mpc

Extension for dolfinx to handle multi-point constraints.
https://jorgensd.github.io/dolfinx_mpc/
MIT License
31 stars 12 forks source link

Fix demo_periodic_gep.py #30

Closed conpierce8 closed 1 year ago

conpierce8 commented 1 year ago

Motivation

In demo_periodic_gep.py, the call signature of assemble_and_solve suggests that any combination of boundary conditions can be applied, i.e.:

assemble_and_solve(["dirichlet", "dirichlet"])
assemble_and_solve(["dirichlet", "periodic"])
assemble_and_solve(["periodic", "dirichlet"])
assemble_and_solve(["periodic", "periodic"])

However, assemble_and_solve provides incorrect eigenvectors for the following boundary condition combinations:

Fixing the Dirichlet-Dirichlet case is unimportant since this case does not demonstrate any features of dolfinx_mpc. Fixing the Dirichlet-periodic case is not very important, since the periodic-Dirichlet case is equivalent and is already implemented correctly. However, the periodic-periodic case should be fixed since it demonstrates the special steps that must be taken in the case when multiple periodic conditions apply to the same dof. In the existing implementation of the periodic-periodic case, incorrect modes are obtained in which the eigenvector is not periodic between points $(0, 0)$ and $(1, 1)$. See example below where the solution has been periodically extended, showing gaps in the solution between adjoining unit cells.

demo_periodic_gep_incorrect

Changes

conpierce8 commented 1 year ago

Related to #21. It seems like the fix introduced in #22 did not fully fix the issue raised in #21.

jorgensd commented 1 year ago

Related to #21. It seems like the fix introduced in #22 did not fully fix the issue raised in #21.

Im fine with the fixes here, just make sure that the CI passes

conpierce8 commented 1 year ago

Im fine with the fixes here, just make sure that the CI passes

Looks like it was failing on Flake8. Should pass now (I hope!)