Make discrete transform plans more compact #1435
Conversation
There was a problem hiding this comment.
Thanks for doing this important refactoring of the innards!
The if-statement jungle isn't something I was proud of but couldn't really figure out how to abstract it nicely in PR #1338 so I left it as-is...
I need to double-check the logic, but maybe someone can confirm.
Poisson solvers are tested to make sure the solution is correct to machine epsilon and are tested for 2nd-order convergence for all topologies on the CPU and GPU (e.g. https://buildkite.com/clima/oceananigans/builds/1435#48103f4c-d724-4e61-9011-5d0b7be92dea/33-268) and all the tests seem to pass so this PR looks good to merge!
Not sure if this is a preferable form, but I thought I'd open a PR in case.
I'd say this indicates preferable form!
charleskawczynski
force-pushed
the
ck/compactify_plans
branch
from
d058bc8
to
ec1bacf
Compare
|
@ali-ramadhan, it looks like the docs are failing ( |
|
@charleskawczynski Ah no just a random error but I reran the Buildkite pipeline out of habit. Feel free to merge! |
I was trying to understand the dims pattern in the branching in
plan_transforms, and ended up re-writing it a bit to understand it. Not sure if this is a preferable form, but I thought I'd open a PR in case. I need to double-check the logic, but maybe someone can confirm.