mdolab / idwarp

IDWarp is a mesh warping package for the MACH framework.
Other
17 stars 29 forks source link

Support latest PETSC #54

Closed A-CGray closed 3 years ago

A-CGray commented 3 years ago

Purpose

Fixing issues with the VecGetValues function so that idwarp works with newer versions of petsc, part of tackling https://github.com/mdolab/docker/issues/83.

We haven't figured out why, but in newer versions of petsc, VecGetValues expects a scalar rather than a single entry array for the index when retrieving a single value. I added some conditional compilation so that we use the correct inputs depending on the petsc version installed and therefore maintain backwards compatibility.

Type of change

What types of change is it? Select the appropriate type(s) that describe this PR

Testing

Testflo tests pass locally with petsc 3.12 and on docker image with 3.15

Checklist

Put an x in the boxes that apply.

ewu63 commented 3 years ago

Does the scalar argument not work for older version of PETSc?

A-CGray commented 3 years ago

Nope, I get this:

getSurfaceCoordinates.F90:22:31:

   22 |       call VecGetValues(Xs, 1, iStart+i-1, coordinates(i), ierr)
      |                               1
Error: Rank mismatch in argument ‘c’ at (1) (rank-1 and scalar)
marcomangano commented 3 years ago

(I was getting extremely confused about the difference between this and #52 , then I looked at the branches - why was that necessary? The commit history looks the same)

A-CGray commented 3 years ago

I think I was being an idiot and couldn't figure out how to push directly to the modlab fork of idwarp so had to push to my own, that PR just merged my fork's update-petsc branch into the mdolab one, this PR is to merge those changes into master

marcomangano commented 3 years ago

alright I see, and ofc you did not squash so to a reviewer they look like "double commits". No worries, looks fine!

A-CGray commented 3 years ago

Hold back on merging for now, I think we can do this in a much simpler way

A-CGray commented 3 years ago

Was able to figure out how to retrieve all necessary values with a single call to VecGetValues this means we don't need to do all that conditional compilation, I kept it in verifyWarpDeriv though as there the call to VecGetValues is part of a much bigger loop that I wasn't confident messing with