-
Notifications
You must be signed in to change notification settings - Fork 0
compiler: Change PETScArray type to PetscScalar #64
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
base: master
Are you sure you want to change the base?
Conversation
…he else statement
compiler: Rework /petsc/iet and PETScArrays
Rework /petsc/iet and PETScArrays
- Ensure that PetscInitialize and PetscFinalize are called only once per script, rather than for each Operator constructed. - Support PETSc command line args
| if isinstance(obj, (AbstractFunction, IndexedData)) and mode >= 1: | ||
| if not obj._mem_stack: | ||
| strtype = '%s%s' % (strtype, self._restrict_keyword) | ||
| strtype = '%s%s' % (strtype, obj._restrict_keyword) |
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.
f-string, we try to update all string to f".. {}" whenever we touch one
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.
I don't think _restrict_keyword should be an object property that seems at the wrong place.
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.
Do you have any other suggestions on where it could go if I need to drop it for certain objects like PETScArrays?
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.
Could it be attached to the class?
tests/test_petsc.py
Outdated
|
|
||
| f = Function(name='f', grid=grid, space_order=2) | ||
| g = Function(name='g', grid=grid, space_order=2) | ||
| f = Function(name='f', grid=grid, space_order=2, dtype=np.float64) |
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.
you don't need to repeat the dtype of it's already the one from the grid.
tests/test_petsc.py
Outdated
| grid = Grid(shape=(11, 11)) | ||
| f = Function(name='f', grid=grid, space_order=2) | ||
| grid = Grid(shape=(11, 11), dtype=np.float64) | ||
| f = Function(name='f', grid=grid, space_order=2, dtype=np.float64) |
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.
dtype not needed
tests/test_petsc.py
Outdated
| f1 = Function(name='f1', grid=grid1, space_order=2) | ||
| f2 = Function(name='f2', grid=grid2, space_order=4) | ||
| f3 = Function(name='f3', grid=grid3, space_order=6) | ||
| f1 = Function(name='f1', grid=grid1, space_order=2, dtype=np.float64) |
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.
same
tests/test_petsc.py
Outdated
|
|
||
| f = Function(name='f', grid=grid, space_order=2) | ||
| g = Function(name='g', grid=grid, space_order=2) | ||
| f = Function(name='f', grid=grid, space_order=2, dtype=np.float64) |
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.
same
tests/test_petsc.py
Outdated
|
|
||
| f1 = Function(name='f1', grid=grid, space_order=2) | ||
| g1 = Function(name='g1', grid=grid, space_order=2) | ||
| f1 = Function(name='f1', grid=grid, space_order=2, dtype=np.float64) |
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.
same
| arrays=None, **kwargs): | ||
| self._target = kwargs.get('target', target) | ||
|
|
||
| petsc_precision = mapper[get_petsc_precision()] |
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.
Nitpick: This dict should probably have a more descriptive name
| """ | ||
| return False | ||
|
|
||
| @property |
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.
I would make this an attribute of the class rather than the object. I.e. have _restrict_keyword = 'restrict' immediately under the docstring.
tests/test_petsc.py
Outdated
| op = Operator(petsc) | ||
|
|
||
| # Check the solve function returns the correct output | ||
| op.apply() |
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.
Nitpick: I think it's probably better if the op.apply() is also within the switchconfig
devito/petsc/utils.py
Outdated
| """ | ||
| petsc_dir = get_petsc_dir() | ||
| petsc_arch = get_petsc_arch() | ||
| variables_path = os.path.join( |
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.
Suggestion, use pathlib instead of chaining os.path commands
794ac3e to
cccb988
Compare
_C_ctypeofPetscArrayobjects toPetscScalar_restrict_keywordto be a property ofBasicobjects, instead of a string defined in theCGenclass. By overriding this property forPetscArrayobjects, we can prevent specific warnings generated by PETSc.TODO: