Conversation
connorjward
left a comment
connorjward
left a comment
There was a problem hiding this comment.
Quick skim. I hope this is useful and not just annoying!
| opc = pc | ||
| appctx = self.get_appctx(pc) | ||
| fcp = appctx.get("form_compiler_parameters") | ||
| fcp = get_appctx(pc).fcp |
There was a problem hiding this comment.
Did you set fcp? It's not a clear name.
There was a problem hiding this comment.
No, it is already attached to the _SNESContext. I'm just switching to getting the compiler parameters from there instead of the appctx because it is a "global" rather than a namespaced (prefixed) thing.
| ctx = get_appctx(pc) | ||
| fcp = ctx.fcp | ||
|
|
||
| appctx = ctx.appctx |
There was a problem hiding this comment.
This is weird. This is saying appctx = get_appctx(pc).appctx
There was a problem hiding this comment.
I agree, this is really annoying. There is the _SNESContext object and the dict object, both of which are called appctx in various places.
_SNESContext.appctxis thedictwe call theappctxin user-code.dmhooks.get_appctxreturns the_SNESContext.PCSNESBase.get_appctxreturnsdmhooks.get_appctx(pc_or_snes).appctxwhich is thedict.
I would be in favour of cleaning up this naming, but it'd touch a lot of code and I don't want to do it unilaterally.
There was a problem hiding this comment.
Sure. I'd be happy for now to just open an issue about it, plus perhaps an explanatory comment.
|
|
||
| if appctx is None: | ||
| appctx = {} | ||
| from petsctools import AppContext |
There was a problem hiding this comment.
Do import at the top level
2 participants
Switch from an un-namespaced python
dictto the namespacedpetsctools.AppContextin this PR: firedrakeproject/petsctools#16