Chi-Squared Gradient implementation.

[sjperkins]

Chi-Squared Gradient, implemented by Marzia Rivi.

[sjperkins]

@marziarivi I'll be taking a look at this soonish.

[marziarivi]

Thanks! Actually it is not working well and I am trying to understand the problem. For sure one problem is connected with the bug of the conjugated visibilities…
Maybe we can have a chat on Skype…

Cheers,
Marzia

On 7 Mar 2016, at 09:50, Simon Perkins notifications@github.com wrote:

@marziarivi I'll be taking a look at this soonish.

—
Reply to this email directly or view it on GitHub.

[sjperkins]

@marziarivi, cc I've merged a large change into master, you're going to have either rebase this branch or merge the changes on the master branch into it.

All of the changes adding nparam in BaseSolver.py will fall away. Instead you'll need to add a

self.register_dimension('nparam', ...)

in the constructor of impl/biro/v4/BiroSolver.py to accomplish the same. Take a look at how the beam_lw dimension is registered in the same constructor.

There are four points that I think need to be addressed:

• This change needs some NumPy test cases to verify the GPU code.
• Would it be possible to implement the X^2 Gradient for Gaussians and point sources? At the moment it seems implemented for sersic sources only.
• Are the X^2 and it's gradient usually computed separately or simultaneously? I'm leaning towards computing the gradient separately since it's quite computationally expensive.
• What effect does the extra compute have on the kernel register count? Is there any spill to local memory?

[marziarivi]

On 15 Mar 2016, at 14:17, Simon Perkins notifications@github.com wrote:

@marziarivi, cc I've merged a large change into master, you're going to have either rebase this branch or merge the changes on the master branch into it.

All of the changes adding nparam in BaseSolver.py will fall away. Instead you'll need to add a

self.register_dimension('nparam', ...)
in the constructor of impl/biro/v4/BiroSolver.py to accomplish the same. Take a look at how the beam_lw dimension is registered in the same constructor.

ok
There are four points that I think need to be addressed:

This change needs some NumPy test cases to verify the GPU code.
Would it be possible to implement the X^2 Gradient for Gaussians and point sources? At the moment it seems implemented for sersic sources only.
This gradient has been computed with respect to the Sersic galaxy shape parameters only. I did it because I need it for my code implementing HMC using Montblanc. In principle you can write a version for the Gaussian galaxies as well but I don’t use/know this model and at the moment I have no time for implementing it. I am still trying to understand if the current one is working well.
Are the X^2 and it's gradient usually computed separately or simultaneously? I'm leaning towards computing the gradient separately since it's quite computationally expensive.
This is the reason why I proposed separate files for this: RimeSumCoherenciesWithGradient.py, including both the chi-squared and the gradient and the original one, so you can choose either to compute the chi-squared only or the the chi-squared and the gradient.
I agree that would be more practical and clear to have the computation of X^2 and gradient separate but on the GPU implementation my solution should be faster because it reduces memory accesses (avoiding to read all the visibilities) and avoids duplication of the single source visibilities computation as the gradient requires them.
What effect does the extra compute have on the kernel register count? Is there any spill to local memory?
Actually, since I have 3 parameters for each source I have to use an array to be stored in the local memory because I will work with many sources up to 10^4 and I don’t think there is enough space in the shared memory. But if you have any suggestion it is welcome!

Anyway you can remove my pull request as I changed something, then If you prefer we can arrange a way to keep this feature separate from the main code or not adding this stuff to the repository at all.

Cheers,
Marzia

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#91 (comment)

[marziarivi]

I’ve merged the gradient branch with the last version of the master and defined ‘nparams’ according to your instructions.

It works but I have the following problem when nparams==0 (which is the default value, i.e. no gradient computed):
File "/scratch/hpcrivi1/MY_MONTBLANC/lib/python2.7/site-packages/montblanc-0.4.0_alpha3_46_gf8880a5-py2.7.egg/montblanc/dims.py", line 163, in check
.format(d=name, gs=gs, e0=E[0], e1=E[1]))
AssertionError: Dimension 'nparams', global size 0, extents [0, 0]

It seems it doesn’t allow to have value zero. Should I define it differently?

[sjperkins]

Add a zero_valid=True flag when registering it, if that doesn't work look at dims.py to see if i got the keyword argument correct.

[marziarivi]

Ok. Now it works!

[sjperkins]

@marziarivi Let me explicitly know here when I can delete the gradient branch. I'll only do so once you've given the command.

[sjperkins]

Deleting this branch as @marziarivi re-implemented in #147

[marziarivi]

Ciao, Ho appena firmato la petizione "Sergio Mattarella: Respingere la nomina di Gian Carlo Blangiardo alla Presidenza Istat - Appello" e vorrei chiederti di aiutarci aggiungendo il tuo nome. Il nostro obiettivo è quello di raggiungere 497 firme e abbiamo bisogno del tuo sostegno. Puoi saperne di più e leggere la petizione qui: http://chng.it/bxfByR9Kwx Grazie! Marzia