devitocodes / devito

DSL and compiler framework for automated finite-differences and stencil computation
http://www.devitoproject.org
MIT License
555 stars 225 forks source link

Copies in operator #66

Closed mloubout closed 7 years ago

mloubout commented 8 years ago

I realized that all inputs to TTI_codegen and Acoustic_codegen are copied e.g :

self.m = DenseData(name="m", shape=self.model.vp.shape, dtype=self.dtype) self.m.data[:] = self.model.vp**(-2)

Ii means that if model is modified, Acoustic won't be making the inversion workflow complicated. WOuld it be possible to do something like self.m = DenseData(name="m", shape=self.model.vp.shape, dtype=self.dtype) self.m.data[:] = self.model.get_m()

where self.model.get_m() would be a pointer to the get_m function so that everytime the operator is called it will run with whatever is vp at the current time. It will allow to separate the model structure (and Shot, rec) fro mthe actual propagator allowing model changes without having to recreate a new operator that wold copy again vp.

vincepandolfo commented 8 years ago

I believe this belongs to https://github.com/opesci/inversion

mlange05 commented 7 years ago

Ok, I think there are two things here: a) Devito wants/needs to own the allocation, so overwriting .data with a pre-defined pointer negates all the data alignment we enforce. As a result we could be working on potentially unaligned data which could harm performance and maybe even correctness. So a better model would be to pass the pre-allocated pointer to the routine that creates model data, eg. self.model.create_m(self.m.data). Alternatively, I believe the initializer function argument on DenseData and TimeData objects is meant to perform exactly this type of in-place data instantiation.

b) I agree that this issue really is an application design problem and therefore belongs in either SLIM or inversion. @mloubout If you agree, I think we can close this issue.

mloubout commented 7 years ago

Agreed. closing