Open jmeyers314 opened 9 years ago
This looks like a very good plan.
Here are couple of further questions:
jmax
, increases. Does this merit consideration of the dictionary or other data structure for storing all this information? Is it possible that looking up saved basis functions could contribute significantly to the computation time of SeriesConvolution.drawImage()
?Just a quick comment on the user interface.
Perhaps I am being overly influenced by our recent discussion about the ChromaticObject interpolation stuff, but an alternative to the user interface that you proposed would be something like
# Direct draw
gal1 = Spergel(nu=0.0, half_light_radius=0.5).shear(g1=0.2, g2=-0.3)
conv1 = galsim.Convolve(gal1, psf)
img1 = conv1.drawImage(nx=32, ny=32, scale=0.2)
# Series draw
gal1.setupSeries(jmax=3)
conv2 = galsim.Convolve(gal1, psf)
img2 = conv2.drawImage(nx=32, ny=32, scale=0.2)
So the basic idea is that it's the same object, but you're changing how you do the internal calculations and image rendering to use the series approximation.
@jmeyers314 , what ever happened with this? I know the initial implementation was merged to master, then there was a bunch more work on this, but then it just kind of stopped.
The next step in implementing the series representation of the Spergel profile (see issue #616 and PR #625) is the implementation of the series basis functions (which for now I'm deeming "Spergelets") and some framework for picking the appropriate coefficients for each Spergelet. I've already more or less got the Spergelets drawable on a local branch, but haven't started the rest of the framework yet. Here's my proposal though:
I think it would be nice if the implementation of the Spergel profile via Spergelets shares most of the same API as the directly implemented Spergel profile. For example:
My current plan to accomplish this, which is pretty close to the plan @mdschneider laid out in #616, is to create a
GSObject
subclassSeries
from whichSpergelSeries
would derive.SpergelSeries
would then have methods:getCoeff((radial_index, azimuthal_index))
, andgetBasisFunc((radial_index, azimuthal_index))
, and also override transformation methods likeshear
,dilate
, etc. to implement these by changing the coefficients of the expansion. Similar toChromaticConvolution
, I would also imagine aSeriesConvolution
subclass ofSeries
with adrawImage
method that does something like:There are at least two potential drawbacks that I can see:
InterpolatedImage
and redraw, I suppose, but that almost certainly destroys the speedup acquired from precomputing convolutions and making everything linear.