Add positivity constraint on the propagator #338
Conversation
| G = None | ||
| h = None | ||
| else: | ||
| G = self.cache_get('shore_matrix_G', key=(self.pos_grid, self.pos_radius)) |
There was a problem hiding this comment.
I would give the cached matrix a more declarative name than shore_matrix_G. Something like shore_matrix_positive_constraint
|
Nice job, I just made a couple comments |
| @@ -108,6 +110,13 @@ def __init__(self, | |||
| square root of the b-value. | |||
| constrain_e0 : bool, | |||
| Constrain the optimization such that E(0) = 1. | |||
| pos_grid : int, | |||
| Grid that define the points of the EAP in which we want to enforce | |||
| positiveness, if None no constrain is imposed. The parameter | |||
There was a problem hiding this comment.
Typos => "positivity", "no constraint"
Garyfallidis
changed the title
| @@ -108,6 +110,13 @@ def __init__(self, | |||
| square root of the b-value. | |||
| constrain_e0 : bool, | |||
| Constrain the optimization such that E(0) = 1. | |||
|
@maurozucchelli when you say it makes it slower can you give us some timings? How much slower and perhaps how much better the result is? Is there anything that can be done to make it faster can the psi matrix be smaller or something? Or can some more things be cached in advance? |
|
@Garyfallidis I made some tests: |
|
Thx! |
Add positivity constraint on the propagator
This pull request add the possibility to enforce positiveness in the EAP in the set of points defined by a grid of a certain radius. The number of points depend on the size of the grid. This option improves the results but makes the fitting a lot slower than without it, so it is better to chose the size of the grid wisely.