Conversation
inducer
force-pushed
the
gmres
branch
2 times, most recently
from
1afaaaa to
ca1a922
Compare
alexfikl
left a comment
alexfikl
left a comment
There was a problem hiding this comment.
Left a few comments, but overall this looks like a good place for it to me.
| class InnerProduct(Protocol, Generic[T]): | ||
| """A :class:`~typing.Protocol` for the inner product used by :func:`gmres`.""" | ||
|
|
||
| def __call__(self, a: T, b: T) -> T: ... |
There was a problem hiding this comment.
| def __call__(self, a: T, b: T) -> T: ... | |
| def __call__(self, a: T, b: T) -> float: ... |
?
There was a problem hiding this comment.
Not really: GPU array contexts return reduction results as device scalars, i.e. arrays. And numpy is forgiving enough that it sort-of fits.
| if (isinstance(x, Number) | ||
| or (isinstance(x, np.ndarray) and x.dtype.char != "O")): | ||
| return np.vdot(x, y) |
There was a problem hiding this comment.
Seeing as we have a NumpyArrayContext now, does this make sense (the isinstance(x, ndarray) part)? Not quite sure where this was used in pytential either..
There was a problem hiding this comment.
It does make sense to me. 😁 This gets used on object arrays that look like [DOFArray, scalar], e.g. when there are scalar degrees of freedom floating around somewhere.
| if show_spectrum: | ||
| ev = la.eigvals(mat) | ||
| import matplotlib.pyplot as pt | ||
| pt.plot(ev.real, ev.imag, "o") | ||
| pt.show() |
There was a problem hiding this comment.
Don't know. It's not meant for serious use because of the inherent inefficiency, so it doesn't matter? And six times out of ten, the reason I'm reaching for this is because the iterative solve died because of some issue in the spectrum. Compromise: _show_spectrum?
2 participants
Since I want access to a solve in grudge, I figured moving GMRES from pytential would be the easiest. Once this is merged, I'll remove the one in pytential (with deprecation blah). This one isn't totally compatible, fwiw: The pytential one had some actx autodiscovery "stuff" that I'll probably have to re-add.
This also brings sumpy's
build_matrixhere.