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

Use SubMatrices to compute eigvals in demo qgbasinmodes #2402

Closed
wants to merge 9 commits into from

Conversation

APaganini
Copy link
Contributor

To compute the eigenvalues more robustly, create submatrices by removing rows and columns associated with degrees of freedom. This trick was suggested (on a different example) by @jandrej , @wence- , @francesco-ballarin and Matt Knepley, among others.

Disclaimer I haven't run the demo to check that it runs because I don't know how to convert an *.py.rst into a *.py I can run.

@jandrej
Copy link
Contributor

jandrej commented Apr 1, 2022

Sweet. I like the complement approach of getting the dofs.

@APaganini
Copy link
Contributor Author

Sweet. I like the complement approach of getting the dofs.

It was Lawrence's idea :)

@wence-
Copy link
Contributor

wence- commented Apr 1, 2022

To compute the eigenvalues more robustly, create submatrices by removing rows and columns associated with degrees of freedom. This trick was suggested (on a different example) by @jandrej , @wence- , @francesco-ballarin and Matt Knepley, among others.

Disclaimer I haven't run the demo to check that it runs because I don't know how to convert an *.py.rst into a *.py I can run.

Run make demos in the demos directory

Copy link
Contributor

@wence- wence- left a comment

Choose a reason for hiding this comment

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

I think this needs a bit of work to be correct in parallel.

demos/eigenvalues_QG_basinmodes/qgbasinmodes.py.rst Outdated Show resolved Hide resolved
@@ -214,7 +223,7 @@ solver::

and we gather the final eigenfunctions ::

eigenmodes_real.vector()[:], eigenmodes_imag.vector()[:] = vr, vi
eigenmodes_real.vector()[ISin.array], eigenmodes_imag.vector()[ISin.array] = vr, vi
Copy link
Contributor

Choose a reason for hiding this comment

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

You should also insert the Dirichlet conditions into these eigenvectors (bc.apply)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this necessary, given that bc = DirichletBC(Vcg, 0.0, "on_boundary")?

@APaganini
Copy link
Contributor Author

Applied most of @wence- suggestions. Now it doesn't run because I don't know how to properly implement ISin = ISbd.complement(s, e)

@wence-
Copy link
Contributor

wence- commented Apr 1, 2022

I probably got the syntax wrong for getting the size of the matrix

@APaganini
Copy link
Contributor Author

APaganini commented Apr 6, 2022

I corrected (s, e), _ = petsc_a.getSizes() into petsc_a.getOwnershipRange(). Now it runs. The result is slightly different from the original version. More specifially, the original output is

Leading eigenvalue is: (1.560678669632054e-13+0.10979244518390797j)

whereas after the modification from this pull request the output becomes

Leading eigenvalue is: (1.1395615354126143e-14+0.10979281297538487j)

The real part is different (but still very close to 0), whereas the imaginary parts differ by -3.67791477e-7. I assume this is fine, but my opinion on this is uninformed.

TODO:

  • Resolve conversation about correct application of DirichletBC (see above)
  • Fix the naming convention, which @wence- said they are rather off from normal Python. Suggestions are welcome.

bcnodes = Vcg.dof_dset.lgmap.apply(bc.nodes).astype(pyop2.datatypes.IntType)
ISbd = PETSc.IS().createGeneral(bcnodes, comm=petsc_a.comm)
s, e = petsc_a.getOwnershipRange()
ISin = ISbd.complement(s, e)
Copy link
Contributor

Choose a reason for hiding this comment

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

In parallel, ISbd.complement(s, e) may rise exception:

File PETSc/IS.pyx:234, in petsc4py.PETSc.IS.complement()

Error: error code 62
[0] ISComplement() at /home/zzyang/workspace/firedrake-gcc/firedrake-mini-petsc-complex/src/petsc/src/vec/is/is/utils/iscoloring.c:815
[0] Invalid argument
[0] Index set must be sorted
[1:execute]

Maybe need add

s, e = petsc_a.getOwnershipRange()
bcnodes = bcnodes[np.logical_and(bcnodes < e, bcnodes >= s)]
bcnodes.sort()

to delete the ghost nodes and sort the array before creating the ISbd.

@dham
Copy link
Member

dham commented Feb 20, 2024

If this is to be done it should be done in LinearEigenSolver. The right thing to do, though, is RestrictedFunctionSpace, which is hopefully coming.

@dham dham closed this Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants