-
Notifications
You must be signed in to change notification settings - Fork 242
compiler: Reorganise PETSc files and add lower_petsc_symbols
#2724
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
lower_petsc_symbols
|
Why is this being merged into the solver parameters branch, rather than the PETSc one? Or are you going to rebase once the solver params branch is merged? |
| from devito.passes.iet.langbase import LangBB | ||
| from devito.symbolics import c_complex, c_double_complex | ||
| from devito.tools import dtype_to_cstr | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does devito.petsc get its own line? I thought the "notional" "Devito style" grouped Devito imports.
(I will reiterate my desire to automate this nonsense!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pick a different filename (setup.py should only be used for installation)
devito/petsc/iet/time_dependence.py
Outdated
| from devito.petsc.iet.utils import petsc_call | ||
|
|
||
|
|
||
| class NonTimeDependent: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a great name
devito/petsc/iet/time_dependence.py
Outdated
| return [] | ||
|
|
||
|
|
||
| class TimeDependent(NonTimeDependent): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above comment! A NonTimeDependent is a type of Something. The convention should be:
- Replace
NonTimeDependentwithSomething - This class should be a
TimeSomething
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please find a proper home for all of these functions (and then one day hopefully someone can delete this file)
| class MainUserStruct(PETScStruct): | ||
| pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this exist?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Find a proper home for these functions while you are refactoring and delete this file.
2d1fd0c to
09a1d20
Compare
6ab8c67 to
e07eb5a
Compare
The bulk of this PR is a cleanup:
petsc/iet/routines.pyhad become too large, so it has been split into four files:New code:
lower_petsc_symbolscalled insidepasses/iet/languages/C.py: This was introduced because, in some cases, new symbols (e.g.x_size,y_size) are injected by theplace_definitionsandplace_castspasses. In PETSc callback functions, such symbols can only be accessed through a struct (viaDMGetApplicationContext). This struct is generated inside thelower_petscpass.lower_petsc_symbolsensures that this struct is rebuilt (and any affected IET is updated) to include the new symbols.Two new iet passes inside
petsc/iet/passes.pywhich are called insidelower_petsc_symbols.