Skip to content

Deprecate passing 'appctx' to solvers.#5265

Draft
connorjward wants to merge 2 commits into
mainfrom
connorjward/proper-appctx
Draft

Deprecate passing 'appctx' to solvers.#5265
connorjward wants to merge 2 commits into
mainfrom
connorjward/proper-appctx

Conversation

@connorjward

Copy link
Copy Markdown
Contributor

Everything can now go in via the options dictionary.

For a seamless deprecation process preconditioners will display a warning if the old appctx is used, and custom preconditioners doing the wrong thing will display a warning too.

Needs firedrakeproject/petsctools#43

Everything can now go in via the options dictionary.

For a seamless deprecation process preconditioners will display a
warning if the old appctx is used, and custom preconditioners doing the
wrong thing will display a warning too.
appctx = self.get_appctx(pc)
fcp = appctx.get("form_compiler_parameters")
prefix = pc.getOptionsPrefix() or ""
fcp = dmhooks.get_appctx(pc.getDM()).get_python_option(prefix, "form_compiler_parameters", None)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We store a handle to these on both the snescontext and in the appctx. It's essentially global data so we should just get it from the snescontext.

Suggested change
fcp = dmhooks.get_appctx(pc.getDM()).get_python_option(prefix, "form_compiler_parameters", None)
fcp = dmhooks.get_appctx(pc.getDM()).fcp

entities = self.sort_entities(dm, axis, dir, divisions=divisions)
except KeyError:
raise KeyError("PlaneSmoother division key %s not provided" % sweep_split[2:])
breakpoint()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
breakpoint()

@JHopeCollins

Copy link
Copy Markdown
Member

Thanks for starting this up again.

See https://github.com/firedrakeproject/firedrake/pull/4526/changes for a bunch more places I found needed changes.

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.

3 participants