Trafo cache #29
Trafo cache #29
Conversation
There was a problem hiding this comment.
Everything looks good, although I'm slightly concerned about what happens for sources larger than the largest size in source_sizes. I would like to see an option that allows the user to set the behavior for sources larger than the largest specified size.
Some examples are
- Do what the current code does, namely, truncate the source.
- Let the source use whatever size box it wants. This is less than ideal, but may be better than forcing the object to be contained in a box that it doesn't fit in.
- Specify a
step_sizethat handles very large sources. For example ifsource_sizes=[5,10,15,30,60,90]andstep_size=50, then objects larger than90would have sizes of90+50n, for integern. Ifstep_sizeisNone, then the current behavior (truncation) would be used.
I know that the user can always just generate this list for his/herself, but this seems like a useful convenience method to protect the user from unexpected behavior.
| L = scipy.sparse.eye(size, k=size) | ||
| global cache | ||
| cache[name][key] = L | ||
| return L | ||
|
|
||
| def getIdentityOp(shape): |
There was a problem hiding this comment.
Good call, I wouldn't have thought to cache this but it probably does take longer than it should for scipy to build these.
| @@ -668,6 +667,16 @@ def _set_edge_flux(self, m, model): | |||
| self._edge_flux[m,2,:] = np.abs(model[:,:,0,:]).sum(axis=0).mean(axis=1) | |||
| self._edge_flux[m,3,:] = np.abs(model[:,:,:,0]).sum(axis=0).mean(axis=1) | |||
|
|
|||
| def _find_next_source_size(self, value): | |||
There was a problem hiding this comment.
I worried about larger sources being truncated in this function.
|
Let open a new issue for the max box size problem |
Transformation matrices get on-the-fly cache to reduce construction cost.
As a side consequence,
Blendneeds a new keywordsource_sizesto know which sizes are allowed according to a predefined list.To work best,
Sourceinitialization needs to respect the same size options, which needs to get fixed when #28 is merged. However, this PR can be merged with master without conflicts.