#365 - changing GSObject and HSM parameters on the fly #385

rmandelb · 2013-04-04T02:38:23Z

This pull request includes some work by Mike on #343 and by me on #365 to make it possible to change some of the GSObject and HSM "magic numbers" on the fly, rather than having to change hard-coded values and recompile each time. These changes should facilitate some of the investigations on various issues like #343, #355, #168, #384, etc.

In order to make this work, there are two new classes (GSParams and HSMParams) that set the magic numbers. These are passed in for example to constructors for GSObjects (in the case of GSParams) or to the moments/shape measurement routines (for HSMParams) if the user desires anything other than the defaults.

Another change from Mike's work on #343 that is included in this pull request is a fix to an issue that was causing drawShoot to use too much memory.

(#365)

(#343) Conflicts: galsim/base.py

(#343)

(#343) Conflicts: include/galsim/SBAiryImpl.h include/galsim/SBDeconvolveImpl.h include/galsim/SBLaguerreImpl.h include/galsim/SBShapelet.h src/SBBox.cpp src/SBDeconvolve.cpp src/SBExponential.cpp src/SBGaussian.cpp src/SBInterpolatedImage.cpp src/SBKolmogorov.cpp src/SBLaguerre.cpp src/SBMoffat.cpp src/SBSersic.cpp

(#343)

…olve (#343)

(#343)

(#365)

reikonakajima · 2013-04-04T10:39:14Z

Compiles and tests on all 4 of my systems---but I suppose I should try running some scripts? Let me check the diff files...

reikonakajima · 2013-04-04T10:44:44Z

...I see, there is a test_hsmparams in there (which I presume is run by the nosetests). I've only skimmed, but the diffs look reasonable so far.

rmandelb · 2013-04-04T12:23:00Z

Yes, test_hsmparams is run by nosetests. Thanks for checking it on all your systems!

On Apr 4, 2013, at 6:44 AM, Reiko Nakajima notifications@github.com wrote:

...I see, there is a test_hsmparams in there (which I presume is run by the nosetests). I've only skimmed, but the diffs look reasonable so far.

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

Rachel Mandelbaum
rmandelb@andrew.cmu.edu
http://www.andrew.cmu.edu/~rmandelb/

(#365)

rmandelb · 2013-04-22T01:52:00Z

Hi all - I think I finished making the changes I promised, but please let me know if you think the tests of HSMParams are still insufficient and I can make even more.

reikonakajima · 2013-04-22T13:30:35Z

Compiles and tests fine on all 4 of my systems. (I'll try to read the dox later today...)

(#365)

reikonakajima · 2013-04-22T20:57:01Z

galsim/psfcorr.py

@@ -379,6 +416,9 @@ def FindAdaptiveMom(object_image, weight = None, badpix = None, guess_sig = 5.0,
                             exception when moment measurement fails.  If set to `False`, then 
                             information about failures will be silently stored in the output 
                             HSMShapeData object.
+    @param hsmparams         The hsmparams keyword can be used to change the settings used by
+                             FindAdaptiveMom when estimating moments; see docstring for psfcorr.py
+                             for more information.


Probably a stupid question: when you say "see docstring for psfcorr.py for more information", do you mean the docstring above? Or the docstring from other sections of this file psfcorr.py? (I'm not sure if it has more information that one might be looking for)

I'm confused, I was replying to this on the web interface when it (plus my reply) seemed to disappear. The quick answer is I mean the docstring for the file itself, up at the top (which you can see using help(galsim.psfcorr). However, the hsmparams info is actually somewhere else now, so I will push an update to this comment to reflect its real location. Thanks for pointing this out.

On Apr 22, 2013, at 4:57 PM, Reiko Nakajima notifications@github.com wrote:

In galsim/psfcorr.py:

@@ -379,6 +416,9 @@ def FindAdaptiveMom(object_image, weight = None, badpix = None, guess_sig = 5.0,
exception when moment measurement fails. If set to False, then
information about failures will be silently stored in the output
HSMShapeData object.

@param hsmparams The hsmparams keyword can be used to change the settings used by

FindAdaptiveMom when estimating moments; see docstring for psfcorr.py

for more information.
Probably a stupid question: when you say "see docstring for psfcorr.py for more information", do you mean the docstring above? Or the docstring from other sections of this file psfcorr.py? (I'm not sure if it has more information that one might be looking for)

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

Rachel Mandelbaum
rmandelb@andrew.cmu.edu
http://www.andrew.cmu.edu/~rmandelb/

I just fixed this. Not only was this line pointing to the wrong location for hsmparams documentation, the pointers in base.py to gsparams documentation were also wrong (because we got rid of the versions that were in the galsim/ directory).

(#365)

rmjarvis · 2013-04-23T02:56:47Z

One thing I'd like to do on this branch before merging is to add the GSParams stuff to config. I'm working on this now. I think it's working. I'm just adding unit tests for it in test_config_gsobject, which will also serve to make sure that each parameter actually does something when changed. i.e. I'm also testing that the results aren't the same as a GSObject built with the default GSParams.

Anyway, I was trying to figure out which demo to add it to (if any), and I was having trouble coming up with something that wouldn't feel too forced. Not that I haven't forced things before just to get them in a demo, of course, but if anyone has an idea for a good place to showcase the gsparams option in a demo, please let me know.

(#365)

… psf and gal objects (#365)

(#365)

rmjarvis · 2013-04-23T18:52:32Z

I think I have the gsparams working in config now. I set it up so that you can specify any non-default parameters either in the image item, in which case they apply to all objects (psf, pix, and galaxy), or you can add them to a particular one of these, or even particular items farther down the chain in the case of composite objects, lists, etc., in which case the changes would just apply to that particular item (or any contained items in the case of composite objects).

I also added a bunch of unit tests about this that check that things change when you change one of the parameters. These are in test_config_gsobject.py. It doesn't attempt to assess the validity of each change. Just that something happened. As discussed, the validity of the various parameters is still an open issue (#343).

Finally, I also aded usage of gsparams in demo7, so please take a look at that, and see if it seems reasonable. I tried to change the parameters in a way that made numerical differences, but where I couldn't see that something was obviously wrong in the resulting images.

rmjarvis · 2013-04-23T18:55:50Z

Rachel,

One of the times I ran the test suite, I got this error:

======================================================================
FAIL: Test that when non-default hsmparams are used, the results change.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Library/Python/2.6/site-packages/nose-1.1.2-py2.6.egg/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/Users/Mike/GalSim/tests/test_psfcorr.py", line 568, in test_hsmparams_nodefault
    assert(dt2 < dt),'Should be faster to estimate shear with lower max_moment_nsig2'
AssertionError: Should be faster to estimate shear with lower max_moment_nsig2

----------------------------------------------------------------------

It happened that there were other things running at the same time, so I think this was just the vagaries of CPU speed on a busy computer. So I disabled the timing tests by default. There is now a parameter near the top of test_psfcorr.py that can turn them back on. But I worry that they will produce spurious failures from time to time otherwise.

Conflicts: galsim/base.py include/galsim/SBSersic.h include/galsim/SBSersicImpl.h pysrc/SBSersic.cpp src/SBMoffat.cpp src/SBSersic.cpp

rmandelb · 2013-04-24T02:53:30Z

I was a bit worried about that, btu reasoned that on my system there differences were enough that it shouldn't be an issue even by chance. I guess I was wrong!

One option would be to do each operation 10x or something like that, and compare the average times over those realizations, so that random glitches should not have such a significant effect. Otherwise, I agree about disabling the time comparisons.

On Apr 23, 2013, at 2:55 PM, Mike Jarvis notifications@github.com wrote:

Rachel,

One of the times I ran the test suite, I got this error:

FAIL: Test that when non-default hsmparams are used, the results change.

Traceback (most recent call last):
File "/Library/Python/2.6/site-packages/nose-1.1.2-py2.6.egg/nose/case.py", line 197, in runTest
self.test(*self.arg)
File "/Users/Mike/GalSim/tests/test_psfcorr.py", line 568, in test_hsmparams_nodefault
assert(dt2 < dt),'Should be faster to estimate shear with lower max_moment_nsig2'
AssertionError: Should be faster to estimate shear with lower max_moment_nsig2

It happened that there were other things running at the same time, so I think this was just the vagaries of CPU speed on a busy computer. So I disabled the timing tests by default. There is now a parameter near the top of test_psfcorr.py that can turn them back on. But I worry that this will produce spurious failures from time to time otherwise.

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

Rachel Mandelbaum
rmandelb@andrew.cmu.edu
http://www.andrew.cmu.edu/~rmandelb/

(#365)

rmjarvis · 2013-04-24T03:52:54Z

One option would be to do each operation 10x or something like that...

Given that people may be sometimes running the tests on machines with many other jobs running, I think it's better to just keep it turned off. Someone working on the hsm code can turn it back on during development to make sure they don't break anything.

rmandelb · 2013-04-24T12:53:34Z

Fine.
Other than my comment on the demo, I'm done reviewing the new bits you added - they look good.

(#365)

#365 - changing GSObject and HSM parameters on the fly

rmandelb and others added 27 commits March 31, 2013 17:35

initial setup of HSMParams struct in .h file

51bc243

(#365)

working HSMParams at C++ level only, no wrapping yet

e3d8a7d

(#365)

wrapped HSMParams, but currently without proper docstrings in pysrc

70cd109

(#365)

added basic hsmparams functionality in psfcorr.py

6f91f23

(#365)

added part of pysrc that I missed originally

4c10674

(#365)

dereferencing null pointers is a bad idea

a6388ae

(#365)

pass through hsmparams appropriately

e18c919

(#365)

added pysrc docstring for HSMParams

e1ccd2c

(#365)

throw HSMErrors everywhere

8360222

(#365)

hsmparams usage example for FindAdaptiveMom

29ae7c0

(#365)

unit tests for HSMParams

aa8433b

(#365)

noted new functionality in CHANGELOG

6e5d050

(#365)

fixed violations of 100-char width rule

7b1029f

(#365)

Fix drawShoot to not use a lot of memory

ba8d6d1

(#343) Conflicts: galsim/base.py

Remove debugging edit

6a157e3

(#343)

Add unit tests of using GSParams

da5c315

(#343)

Add docs about GSParams

bafdb76

(#343)

Update docs about gsparams wrt wrapper classes: Add, Convolve, Deconv…

67a0113

…olve (#343)

fixing a random typo

4f12aea

(#343)

Use new gsparams object for alias_threshold needed by OpticalPSF

7816692

(#343)

sterner warning

dbab9d2

(#343)

Make GSParams members writable.

b190892

(#365)

Remove the catch(char*) bit

5140243

(#365)

updated CHANGELOG for 343 stuff

3a2e055

(#365)

make hsmparams members writable

f9ed486

(#365)

typo

c653bc6

(#365)

beefed up HSMParams tests

a424b42

(#365)

rmandelb and others added 5 commits April 22, 2013 13:29

changed xvalue_accuracy, kvalue_accuracy in Lanczos interpolant

82aaee9

(#365)

documentation tweaks for interpolants

01185e7

(#365)

python-level documentation for interpolants

a064358

(#365)

dialed back relation between tol and accuracy params for Lanczos

ace9c96

(#365)

formatting

f47cead

(#365)

reikonakajima reviewed Apr 22, 2013
View reviewed changes

clarified where documentation for GSParams and HSMParams can be found

acc49df

(#365)

rmjarvis added 6 commits April 23, 2013 01:20

Enable gsparams options in config

8e485bd

(#365)

Add abillity to set gsparams in image config item to apply equally to…

a43282f

… psf and gal objects (#365)

Add gsparams capabilities to demo7

51a7135

(#365)

More unit tests related to gsparams

f4a1990

(#365)

By default, don't run timing tests in test_psfcorr.

e0239b2

(#365)

Merge branch '#365' of github.com:GalSim-developers/GalSim into #365

245ae29

Merge branch 'master' into #365

d881dfb

Conflicts: galsim/base.py include/galsim/SBSersic.h include/galsim/SBSersicImpl.h pysrc/SBSersic.cpp src/SBMoffat.cpp src/SBSersic.cpp

typos

b582bef

(#365)

rmjarvis added 2 commits April 24, 2013 09:43

Clarify where to find default values of gsparams in demo7

3457a92

(#365)

Merge branch '#365' of github.com:GalSim-developers/GalSim into #365

346cac6

rmjarvis added a commit that referenced this pull request Apr 24, 2013

Merge pull request #385 from GalSim-developers/#365

2d1f28c

#365 - changing GSObject and HSM parameters on the fly

rmjarvis merged commit 2d1f28c into master Apr 24, 2013

rmandelb mentioned this pull request Apr 24, 2013

drawing a 4800 x 4800 pixel image #233

Closed

rmandelb deleted the #365 branch July 4, 2014 14:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#365 - changing GSObject and HSM parameters on the fly #385

#365 - changing GSObject and HSM parameters on the fly #385

rmandelb commented Apr 4, 2013

reikonakajima commented Apr 4, 2013

reikonakajima commented Apr 4, 2013

rmandelb commented Apr 4, 2013

rmandelb commented Apr 22, 2013

reikonakajima commented Apr 22, 2013

reikonakajima Apr 22, 2013

rmandelb Apr 22, 2013

rmandelb Apr 23, 2013

rmjarvis commented Apr 23, 2013

rmjarvis commented Apr 23, 2013

rmjarvis commented Apr 23, 2013

rmandelb commented Apr 24, 2013

FAIL: Test that when non-default hsmparams are used, the results change.

rmjarvis commented Apr 24, 2013

rmandelb commented Apr 24, 2013

#365 - changing GSObject and HSM parameters on the fly #385

#365 - changing GSObject and HSM parameters on the fly #385

Conversation

rmandelb commented Apr 4, 2013

reikonakajima commented Apr 4, 2013

reikonakajima commented Apr 4, 2013

rmandelb commented Apr 4, 2013

rmandelb commented Apr 22, 2013

reikonakajima commented Apr 22, 2013

reikonakajima Apr 22, 2013

Choose a reason for hiding this comment

rmandelb Apr 22, 2013

Choose a reason for hiding this comment

rmandelb Apr 23, 2013

Choose a reason for hiding this comment

rmjarvis commented Apr 23, 2013

rmjarvis commented Apr 23, 2013

rmjarvis commented Apr 23, 2013

rmandelb commented Apr 24, 2013

FAIL: Test that when non-default hsmparams are used, the results change.

rmjarvis commented Apr 24, 2013

rmandelb commented Apr 24, 2013