-
Notifications
You must be signed in to change notification settings - Fork 7
/
todo_and_bugs-to-fix.txt
2037 lines (1608 loc) · 85.1 KB
/
todo_and_bugs-to-fix.txt
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
638
639
640
641
642
643
644
645
646
647
648
649
650
651
652
653
654
655
656
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
689
690
691
692
693
694
695
696
697
698
699
700
701
702
703
704
705
706
707
708
709
710
711
712
713
714
715
716
717
718
719
720
721
722
723
724
725
726
727
728
729
730
731
732
733
734
735
736
737
738
739
740
741
742
743
744
745
746
747
748
749
750
751
752
753
754
755
756
757
758
759
760
761
762
763
764
765
766
767
768
769
770
771
772
773
774
775
776
777
778
779
780
781
782
783
784
785
786
787
788
789
790
791
792
793
794
795
796
797
798
799
800
801
802
803
804
805
806
807
808
809
810
811
812
813
814
815
816
817
818
819
820
821
822
823
824
825
826
827
828
829
830
831
832
833
834
835
836
837
838
839
840
841
842
843
844
845
846
847
848
849
850
851
852
853
854
855
856
857
858
859
860
861
862
863
864
865
866
867
868
869
870
871
872
873
874
875
876
877
878
879
880
881
882
883
884
885
886
887
888
889
890
891
892
893
894
895
896
897
898
899
900
901
902
903
904
905
906
907
908
909
910
911
912
913
914
915
916
917
918
919
920
921
922
923
924
925
926
927
928
929
930
931
932
933
934
935
936
937
938
939
940
941
942
943
944
945
946
947
948
949
950
951
952
953
954
955
956
957
958
959
960
961
962
963
964
965
966
967
968
969
970
971
972
973
974
975
976
977
978
979
980
981
982
983
984
985
986
987
988
989
990
991
992
993
994
995
996
997
998
999
1000
*** CURRENT WORK
[ ] Add multimfit code into main repo
[x] Update model_object.cpp
[X] Update function_object.h/cpp
-- SetImageParameters and AdjustParametersForImage
-- extra image-parameters-related data members
[ ] Update all image functions!
-- individual AdjustParametersForImage method overrides
[x] Update standard functions
[ ] LATER: update extra functions
[X] Check that imfit & makeimage still work
[X] Update add_functions.cpp
[ ] May need to update defintion or calls to AddFunctions() in PyImfit
-- we added optional final (optional) parameter globalFuncFlags
[X] Check that imfit & makeimage still work
[X] Continue trying to compile makemultimages
[X] Copy in tests and test files from multimfit
[X] tests for makemultimages
[X] tests for multimfit
[ ] Update distribution_manifest.py
[ ] Update test files for regression tests
[ ] Copy extra functions from imfit_hg to imfit
-- i.e., those functions that aren't in the repos
[ ] Investigate ModelObject::GetImageOffsets -- two different versions of this function!
** BUGS TO FIX:
[X] libimfit.a and GetPhysicalCoreCount()
-- pyimfit fails to import bcs
ImportError: dlopen(/Users/erwin/coding/pyimfit/pyimfit/pyimfit_lib.cpython-310-darwin.so, 0x0002): symbol not found in flat namespace (__Z20GetPhysicalCoreCountv)
This is presumably bcs we aren't including count_cpu_cores.cpp in the object list
for compiling libimit.a
[x] Figure out where GetPhysicalCoreCount is called
-- convolver.cpp
-- used in _main.cpp to populate options->maxThreads, passed to SetupModelObject()
[ ] Figure out where to add this to PyImfit code
SetupModelObject(options, ...)
newModelObj->SetMaxThreads(options->maxThreads);
pyimfit_lib.pyx
ModelObjectWrapper.setMaxThreads
[ ] where does this get called?
** NEAR-TERM FUTURE TODO:
[ ] Add conditional compilation of Ctrl-C handling (use of stopSignal_flag)
-- currently, this causes import failure of pyimfit:
ImportError: dlopen(/Users/erwin/coding/pyimfit/pyimfit/pyimfit_lib.cpython-310-darwin.so, 0x0002): symbol not found in flat namespace (_stopSignal_flag)
[x] ID places where this is used
-- imfit_main.cpp, definitions.h, solvers/ DESolver.cpp, mpfit.cpp, nlopt_fit.cpp,
nmsimplex_fit.cpp
[x] Add flag to SConstruct, used when compiling library
[ ] Add #ifdef boundaries for use of stopSignal_flag
#ifndef NO_SIGNALS
[x] definitions.h
[x] imfit_main.cpp
[x] solvers/DESolver.cpp
[x] solvers/mpfit.cpp
[x] solvers/nlopt_fit.cpp
[x] solvers/nmsimplex_fit.cpp
[ ] Test compilation and import in pyimfit
[ ] Start using Github Actions for Continuous Integration
-- since Travis CI is not really available to us any more
[X] Get basic CI working
[ ] See about including regression tests
[ ] Online documentation: add individual web pages for tricky image functions
-- for image functions which are particularly complex or tricky to use
(spiral arms! ExpDisk3D), add pages with notes and figures and recommendations
[ ] Test use of FFTW "measure" option for plans
-- can we get meaningful speedups for large image convolution?
-- command-line flag
-- modify model_object.cpp to pass doFFTWMeasure to psfConvolver->DoFullSetup()
-- do some timing tests
[later: modify ]
[ ] Read FFTW docs to see what we need to add
[ ] Figure out what code to test
-- psfconvolve?
[ ] Add option to skip initial fit in bootstrap resampling
-- just use input config file (ideally using values from an actual fit)
as starting values for all fits
-- possible useful for cluster jobs, where we want to do a few iterations
per job and merge them all later (so extra time replicating best fit for
each job is wasted)
[ ] add "--bootstrap-only" ("--skip-fit"?) option
[ ] locate places in main() to modify
[X] Add ability to handle Ctrl-C by exiting gracefully and printing current
best fit (and/or write it to a file)
https://en.cppreference.com/w/c/program/signal
-- We will want to have a compilation flag to turn this on/off (e.g., probably off
when compiling library for PyImfit, until we figure out how to handle *that*)
-- Possible approach:
-- global variable or singleton object with "halt" flag (starts off
set = false); if called, signal-handler function sets this to true
Probably simplest to use C solution of
#include <signal.h>
volatile sig_atomic_t stopSignal_flag = 0;
defined in one of our header files that's included by mpfit.cpp,
nmsimplex_fit.cpp, etc.
If the signal-handler function is called, it sets stopSignal_flag = 1.
-- NM (or other NLopt) solver
-- inside myfunc_nlopt(), check state of halt flag; if true, then
call NLopt function nlopt_force_stop()
https://nlopt.readthedocs.io/en/latest/NLopt_Reference/#forced-termination
-- LM
Need to add code to mpfit.cpp to check state of halt flag, and
terminate safely if it's true. (Should be fairly straightforward;
treat this as xxx.)
Maybe do *not* attempt to compute/return parameter error estimates?
-- DE
same as for LM, basically
If fit was stopped by Ctrl-C, print current state and save to *special*
output file (we don't want a valid output file from a previous stage to
be overwritten unless the fit was actually successful).
[X] Write simple C++ program to test signal handling
-- ~/coding/testing/interrupt_test.cpp
[X] Add interrupt-handling code to mpfit.cpp
[X] Add interrupt-handling code to DESolver.cpp (and/or diff_evoln_fit.cpp)
[X] Add interrupt-handling code to nmsimplex_fit.cpp
[X] Add interrupt-handling code to nlopt.cpp
[X] Modify code in imfit_main.cpp to save parameters to alternate file *if*
user interrupted the fit
-- check status of stopSignal_flag
[ ] Compilation with Vagrant VMs
[x] Test current default (shared-library) compilation
vagrant-$ scons -c ; scons makeimage && ./do_makeimage_tests
vagrant-$ scons imfit && ./do_imfit_tests
[x] Test current --static compilation
vagrant-$ scons -c ; scons --static makeimage && ./do_makeimage_tests
vagrant-$ scons --static imfit && ./do_imfit_tests
[ ] Set up 32-bit Vagrant VM for testing
[ ] Test imfit-1.8.0-linux-32.tar.gz on the 32-bit VM
[ ] Update documentation with new functions
[X] Add description of GaussianRingAz to imfit_howto
[X] Add description of FlatBar to imfit_howto
[X] Double-broken exponential (for Nacho)
[X] Figure out what's causing minor-axis discontinuities
-- initial creation of ell=0.5 image shows good behavior along major
axis, strange behavior at faint levels along minor axis
[X] Try generating 1D minor-axis image
[X] Add printing code to func_double-broken-exp.cpp (at what radius
do we switch to pure exponential calculation?)
[X] Generate 1D minor-axis image
$ ./makeimage config_makeimage_double-broken-exp_1d-minoraxis.dat -o modelimage_doublebroken_1d.fits
imdatamodel_mb1d = fits.getdata("/Users/erwin/coding/imfit/modelimage_doublebroken_1d.fits")
clf();semilogy(imdatamodel_1d[0,:], 'ko', ms=2)
scaledR1 > 100 at |r| >= 152 (and for |r| < 152, not)
assuming q=0.5, this is r >= 76
[X] Generate large-radii values (from Python code) for unit tests
[X] Add fixed code from Python to double-broken-exp.cpp
[X] Run unit tests
[X] Check 1D minor-axis image, using commands above
[ ] Update mp_par struct (params_struct.h) to class with copy constructor
-- this is because we do a fair amount of laborious field-by-field copying
in (at least) multimage_main.cpp, which would be simplified
if we could just do, e.g., new_mp_par = imageDescParamLimits[i]
-- places where this might be useful:
multimfit_main.cpp
ModelObject::AddParameterInfo
[ ] Improvements to imfit.py:
[ ] Test image convolution with overpsf_scale = even number
-- e.g., current WFC/IR empirical PSF images are 101x101 with 4x4 oversampling;
PSF center is at 51,51, like it should be
http://www.stsci.edu/hst/wfc3/analysis/PSF
-- some of the (outer) pixel values are slightly negative...
[ ] Negative pixels?
[ ] Try convolving model image with PSF image as-is
[ ] Try convolving model image with PSF image that has had < 0 pixels set = 0
[ ] Extract sample WFC/IR PSF from empirical-PSF file
[ ] Find/make a good WFC3/IR galaxy image
[ ] Trial fits with regular (TinyTim?) PSF
[ ] Trial fits with oversampled PSF
[ ] Trial fits with
[ ] Check input: Catch cases of oversampled regions that lie or extend outside data
image (accounting for data image subset, if any)
-- do we check in _main.cpp (where we usually catch errors in user input, and where
in principle we know all the original dimensions and specifications), or in
SetupModelObject (more centralized, with less code duplication, but where we don't
know original-data-image coordinates), or in ModelObject::AddOversampledPsfInfo
(more centralized, etc., but we may not know data image dimensions)
-- maybe best to do this in main(), via a call to new method in PsfOversamplingInfo
(note that we already pass X0_offset,Y0_offset when setting up PsfOversamplingInfo)
[ ] Check vectorization of mpfit.cpp
-- $ g++-7 -o solvers/mpfit.o -c -O3 -g0 -msse2 -mavx -fopt-info-vec-optimized -std=c++11 -fopenmp -DANSI -DUSING_SCONS -DFFTW_THREADING -DUSE_OPENMP -I. -I/usr/local/include -Icore -Isolvers -Icdream -Icdream/include -Ifunction_objects -Ifunction_objects_1d -Iprofile_fitting solvers/mpfit.cpp
[ ] Compile code with -fopt-info-vec-optimized to see what was & wasn't vectorized
[ ] Identify large loops (e.g, for i < m) that weren't vectorized
[ ] Refactoring of mpfit.cpp
[X] Go through code and check which variables declared as "static <type> = y" can
be redeclared as "const <type> = y"
-- clearer indication to reader that these are *constants*, not changeable
persistent variables; possibly allows compiler to be a little more efficient
if it knows certain "variables" are really constants?
[ ] MCMC program: "imfit-mcmc"
[X] Flag error and quit when config file has unconstrained parameters
-- i.e., parameters with no constraints is OK for L-M or N-M fits, but
not for DE fits and *not* for MCMC analysis
-- current code apparently converts unconstrained parameters into 0,0 limits
[X] Locate code which checks for missing parameter constraints in DE mode
-- DiffEvolnFit in diff_evoln_fit.cpp
checks parameterLimits input object; prints error message and
returns -2 if one or more limits are missing
[X] Locate where we might check parameterLimits in MCMC code
[X] Implement check for missing parameter limits
[ ] Feedback from dream()
[ ] Detect failed opening of output files, complain and exit
[ ] Detect failed write to output files, complain and exit?
[x] Return values indicating: convergence vs max-iterations
[x] Record and report cumulative number of likelihood evaluations
[ ] Print (and maybe report) total number of iterations
[ ] Possible speedup for dream(): delay writing of output until end
of process
-- currently, dream() writes output once per generation for each chain.
Possibly there is some cleverness in the streams output where things are
buffered until later, but if not, we might gain some speedup (esp. on
machines with traditional hard disks) if we delay writing the chains
until the end.
[ ] No-convergence-test mode
-- flag which tells dream() to ignore results of convergence tests and just
keep on going until p->maxEvals is reached.
-- "--ignore-convergence" ?
[x] Better headers for MCMC output
[x] Look for code that defines bootstrap output header
[x] Adapt code for bootstrap output header to MCMC output
[x] Add printing of input parameter file to header
[x] Create mcmc main.cpp as copy of imfit_main.cpp
[x] Make copy
[x] Change internal strings describing name
[x] Remove command-line code specifying fit algorithm
[x] Remove command-line code specifying fit parameters (e.g., tolerance)
[x] Remove code in main() which calls fitting algorithm
[x] Remove bootstrap-related code in main()
[x] Create target in SConstruct file for mcmc program
[x] Copy existing makeimage code
[x] Modify code to use new program name
[x] Create simple regression test for mcmc program
[x] Identify small test image (e.g., exponential, very small image)
[x] Do test run of MCMC code to ensure output look sensible
[x] Set up run of MCMC code using fixed RNG seed
[x] Set up do_mcmc_test.sh
[x] Test run of do_mcmc_test.sh
[x] Add to regression tests for imfit-mcmc
[x] Add some error-detecting regression tests
[x] Link/copy/add cdream code
[x] 1. create symlink to cdream code
[x] 2. create new subdirectory holding cdream code (and add code to repo)
[x] Options struct for mcmc program
[x] Add CDREAM-related elements from profilefit_mcmc
[x] Add command-line processing code to fill in CDREAM-related elements
[x] Add cdream-related stuff to main
[x] Add declaration and setup of dream_pars struct
[x] Add code passing command-line options to dream_pars
[x] Cleanup of main
[x] Remove parameterInfo-related code
[x] Command-line options for mcmc program
[x] Locate image, model used for bootstrap demo in Erwin 2015
/Users/erwin/Documents/Working/Papers-Finished/Paper-imfit/image_tests/sersictest_lowsn200.fits
~/coding/imfit/imfit -c config_imfit_sersic1.dat sersictest_lowsn200.fits --mlr --bootstrap 500 --save-bootstrap bootstrap_out_lowsn200_modcash.dat
Both files copied to ~/coding/imfit
./imfit -c config_imfit_sersic1.dat sersictest_lowsn200.fits --mlr --bootstrap 500 --save-bootstrap bootstrap_out_lowsn200_500.dat
t = 20.0 sec
./imfit-mcmc -c config_imfit_sersic1.dat sersictest_lowsn200.fits --mlr -o mcmc_out_lowsn200
t = 353 sec
colnames, d_mcmc_lowsn200 = MergeChains('mcmc_out_lowsn200', last=5000)
[x] Clean up/rationalize output chain files
[x] "gen" is currently useless cycling of 0,...9,
[ ] replace C++ streams with stdio?
[x] replace C++ streams with stdio in dream_initialize.cpp
[x] replace C++ streams with stdio in dream_pars.cpp
[ ] replace C++ streams with stdio in dream.cpp
[x] replace C++ streams with stdio in gelman_rubin.cpp
[x] replace C++ streams with stdio in gen_CR.cpp
[ ] Command-line option to do N rounds of image generation to estimate time/model?
--estimate-time = flag to do time estimate (including, by default, 1 round of
model-image-generation)
--timing <N> = optional command specifying number of model-image-generation
rounds to do (as in makeimage)
-- Print summary of estimated run time
minimum time = t_model * nBurnin * numChains
maximum time = t_model * maxEvals * numChains
Note that main loop in dream.cpp is
for (int t = prevLines + 1; t < p->maxEvals; ++t) {
which is guaranteed to run until
t >= burnInStart + p->burnIn
where burnInStart is 0 by default (and only gets reset if the algorithm
re-enters burn-in, so we can safely assume that the *minimum*
[ ] Python script to convert user-specified line in an MCMC output file to
makeimage/imfit config file
-- Also a version to do same with bootstrap output?
-- Internal code to hold functions+parameters in form similar to Andres' code??
[x] Modify dream.cpp: Experiment with converting every-5th-generation code in dream.cpp (roughly,
lines 205--225) to be more like what Vrugt et al. do
-- Vrugt et al. seem to recommend setting gamma = 1.0 every 5th generation
-- Code in dream.cpp
[x] remove lines ~ 205--225 ("if (gammaGeneration++ == 5) { ... }")
[x] modify line ~ 269 ("gamma = 2.38/sqrt(2.0*updateDim[i]*delta);")
to set gamma = 1.0 if t % 5 == 0
[ ] Modify dream.cpp: Add beta_0 parameter from Vrugt 2016?
-- beta_0 is an optional scaling of the normal gamma: gamma' = beta_0 * gamma
By default, beta_0 = 1.0; Vrugt 2016 suggests beta_0 between 0.25 and 0.5 can
be useful for complicated, multi-dimensional posteriors
[] Add beta_0 parameter to parameter struct
[] modify line ~ 269 ("gamma = 2.38/sqrt(2.0*updateDim[i]*delta);")
to be something like "gamma = beta_0 * 2.38/sqrt(2.0*updateDim[i]*delta);"
[] Test beta_0 = 0.25 and 0.5 to see if it helps or hinders us
[ ] Modify dream.cpp: Add proper handling of parameter bounds ??
-- currently, dream.cpp uses parameter bound by setting likelihood = -INFINITY
if proposal variable value is outside bounds (thus rejecting the proposal)
-- see p.269 of Vrugt 2016 for options on handling parameter bounds
[ ] Optional linking of component orientations within a function set:
PA [position angle], INCLINATION [for 3D objects]
In the individual-function parameter lists, we use "link=NAME", where
NAME = one of the allowed function-set parameters (i.e., PA or INCLINATION)
E.g., a function set consistent of a Gaussian and an Exponential with
the exact same PA:
X0 40 35,45
Y0 50 45,55
PA_set 15 0,30
FUNCTION Gaussian
PA link=PA_set
ell 0.3 fixed
I_0 100
sigma 1.0
FUNCTION Exponential
PA link=PA_set
ell 0.5 0.2,0.8
I_0 10 0,50
h 100 50,200
Need some way of tagging parameter vector so we know which individual-function
parameters are duplicated
-- I.e., *remove* those parameters from the main parameter vector sent to
the solver or MCMC; have logic in ModelObject which can copy PA_set
value
Note that these are similar to but also *orthogonal* to the image-description
parameters; perhaps we can use some of the same logic for assembling and
disassembling parameter vectors
Conceptually, we have *set parameters* -- X0,Y0,PA_set,inc_set --
and function parameters. All functions in a set already refer to X0,Y0 in the
set parameters
[X]How do we coordinate PA between 2D and 3D image functions?
OK, our standard definition for a 3D function's line of nodes is
the *same* as for 2D functions, so we're OK there
What about PA for e.g. GaussianRing3D or FerrersBar3D? For now, don't
worry about it...
[-]Add SetPA() method to FunctionObject, which overrides any internal extraction
of PA from main parameter vector?
-- this implies that we *know* which parameters are PA-related, which
is maybe too much specialization (or too much information in ModelObject
vs in the FunctionObjects)
OR:
[-]Always pass in X0,Y0,PA,inc to all functions; let functions decide what to
do with them. (Needs a way of telling functions that they're supposed to use
set PA and/or inc when setting them up.)
-- We would still need logic in main to remove unneeded PA and inc parameters
from the main parameter vector, and logic in ModelObject to add in (dummy)
values to the parameters passed to the FunctionObjects ...
[ ] Separate the code which does model-image-generation timing into separate file
(so it can be used by both makeimage and imfit-mcmc)
[-] Implement Kahan summation in ModelObject::FindTotalFluxes
-- http://stackoverflow.com/questions/18013345/openmp-parallel-for-reduction-delivers-wrong-results
[x] Compute some current flux outputs (e.g., including correct total flux for Exponential)
[x] Implement Kahan summation
[x] Compare results with/without Kahan summation
RESULT: for case of simple Gaussian model, Kahan summation took 2--3 times as long
and produced *no* difference in calculated flux (at 10^-5 level), even for a
very extended Gaussian.
[..] Add "const" specifiers to input parameters of functions, as appropriate
Done through: oversampled_region
[] Possible improved error reporting
[X] When negative pixel values are encountered during vetting of input data image,
mention possibility of missing sky parameter
[..] Spline-interpolation image function
Current preliminary code: function_objects_1d/func1d_spline.cpp/h
-- radial SB profile (usually exponential, Sersic, etc.) is spline interpolation
using a small set of r, I values. The I values are free parameters of the fit;
possibly the r values could be also.
-- function has the usual PA, ellipticity parameters to define r_scaled;
optionally c_0 as well for boxy/disky isophotes
-- possible way to handle changing radii for interpolation points:
when parameter vector is passed to image function in Setup(),
*sort* the parameters by radius, then do interpolation with the sorted
vector of r,I
-- Use optional parameter to set maximum number of possible interpolation
points?
-- would require changing modifying SetExtraParams to set up
parameter names vector properly, reset nParams; maybe also modifiy
GetParameterNames?
**[..] Spline-interpolation of variable ellipticity profile
-- See notes/notes_for_variable-ell-pa.txt
[X] Locate useful galaxy to experiment on (should have smoothly varying ell --> Abell 150 BCG
but ~ constant PA; ideally Sersic rather than Core-Sersic)
[X] ? Determine PSF
[X] Trial standard Sersic fit
[X] Create preliminary image function using Sersic + linear ellipticity interpolation
[X] Variable number of parameters (= variable number of interpolation points)
-- In imfit's main(), we start figuring out how many free parameters there are
after calling AddFunctions; we also compare total number of parameters with
number of input parameters (from config file)
-- Can we tell VarEll function to set its number of parameters after getting
the optional-params input [via call to its SetExtraParams method] so that it
tells the rest of the program how many parameters it *really* needs?
[x] OK, SetExtraParams is called from within AddParameters, so that's good
main: theModel->GetNParams();
ModelObject: return nParamsTot;
ModelObject::DefineFunctionSets
nParamsTot += nFunctionParams + 2*nFunctionSets;
ModelObject::AddFunction
nNewParams = newFunctionObj_ptr->GetNParams();
paramSizes.push_back(nNewParams);
nFunctionParams += nNewParams;
OK, looks like it will work:
[X] Modify SetExtraParams() to set the number of parameters
-- NOTE the following in constructor: nParams = N_PARAMS;
nParams is used:
Constructor -- to set up parameterLabels array
FunctionObject::GetParameterNames, GetParameterUnits, GetNParams
[ ] Enable correct saving of optional params (i.e. N)
-- <functionObject>.GetExtraParamsDescription
-- currently, best-fit parameter file looks like this (missing the parameter line!)
FUNCTION Sersic_VarEll
OPTIONAL_PARAMS_START
OPTIONAL_PARAMS_END
[ ] Test fitting of Abell 150 BCG image
[ ] Modify --list-parameters output so that it includes OPTIONAL_PARAMS_START/OPTIONAL_PARAMS_END
blocks
-- PointSource, PointSourceRot, Sersic_VarEll
-- how do we specify (default?) values? do we need to?
-- should we include a comment reminding the use that these do not need to
be included?
[] Modify handling of oversampled PSF to catch case where user wants entire
image oversampled
-- e.g., if total number of pixels in main + oversampled region (model images,
including edge-padding for PSF) > number of pixels in hypothetical oversampling
of entire image ==> switch to oversampling of entire image
-- need to test this to make sure time spent isn't wrongly estimated (e.g.,
actual time with oversampling includes FFT + downsampling)
-- add option for user to specify this directly?
e.g. "--overpsf_region all"
[X] Combine code for generating output header
Currently: lines 329--335 and also 369--375 of print_results.cpp
-- generate in main() as list of strings?
-- would allow us to remove "string& programName, int argc, char *argv[]" from
interface of SaveParams() and SaveParams2()
[..] Reorganize directory structure:
[x]src/core
[x]src/solvers
[x]src/function_objects
src/function_objects_extra -- for specialized, non-public stuff (n4762 funcs, experimental stuff, etc.)
[x]src/function_objects_1d
[x]src/profilefit -- for all the 1D stuff (except func1d_*)
src/utilities ?
src/extra ? -- for timing_main.cpp
src/model_object ? -- for model_object convolver oversampled_region downsample
[but note that Convolver is used by psfconvolve_main.cpp
[x]tests/ -- regression tests, etc.
[x]unit_tests/
[x]docs/ -- where Doxygen input files go
html -- created by Doxygen
latex -- created by Doxygen
[] Possibly create SConstript files for subdirectories -- see
http://stackoverflow.com/questions/8810418/scons-setup-for-hierarchical-source-but-single-target
[or maybe not, since that's getting into "recursive Makefile considered harmful"
territory, even if SCons isn't necessarily afflicted with the same problems]
[] Refactor print_results.cpp to be simpler and less mangled-from-mpfit
[] Look into possible use of FFTW++ library for convolution
-- C++ wrapper around FFTW, with added optimization for "implicit zero-padding"
of convolutions
[] Optional convolution with charge-diffusion kernel?
[] Ability to update ModelObject with new data/error/mask/PSF images
-- Mainly so we can be more flexible with Python wrapper
ModelObject does two main things:
Computes a model image (w/ optional PSF convolution)
-- optionally computes individual-function images, etc.
Compares model image with data image and computes fit statistic
Model specification:
General model: (e.g., most of makeimage config file)
list of FunctionObjects
function-block indices
parameter vector
Image-specific details:
output image size/shape -- may be set by data-image size
[PSF image]
[oversampled PSF images -- including region specifications]
Things needed for fitting an image, including statistical computations:
data image (nCols, nRows)
image characteristics: gain, readnoise, original_sky, exptime, ncombined
[error image]
[mask image]
Possible updates user might want
New Data and/or Error and/or Mask image
New Data --> updated model image and internal weight sizes?
--> new psfConvolver if PSF being used
New Error --> update internal weight vector
ModelObject::AddErrorVector is "re-entrant"
weightVector is freed *if* already allocated
weightVector then points to external error image
New Mask --> update internal weight vector
maskVector then points to external mask image (NOT allocated/dealloc. w/in ModelObject)
New PSF (no PSF; add PSF)
If same size as old, we just need to update Convolver objects
If different size, we *also* need to change model image size
Switch statistical mode (chi^2 <--> MLR, data-based chi^2 <--> model-based, ...)
IF use-model-errors
Dependencies:
(Output) model image size affects:
nModelVals, nModelColumns, nModelRows
modelVector
oversampledRegions [via nModelColumns, nModelRows]
data image size affects:
nDataVals, nDataColumns, nDataRows
weightVector, extraCashTermsVector, maskVector
outputModelVector, deviatesVector, residualVector, standardWeightVector
[and, if doing fits, model image size, above!]
PSF image size affects:
psfInterpolator, psfConvolver, oversampledRegion objects
[and, of course, model image size, above!]
IMAGE VARIABLES AND OTHER ARRAYS:
Assume that arrays are allocated and freed by ModelObject unless stated
otherwise
<> modelVector
allocated in ModelObject::SetupModelImage; used all over the place
RETURNED by ModelObject::GetModelImageVector
<> outputModelVector -- modelVectorAllocated is set
allocated in ModelObject::GetSingleFunctionImage and in
ModelObject::GetModelImageVector *if* convolution is done
RETURNED by ModelObject::GetModelImageVector
RETURNED by ModelObject::GetModelImageVector (which fills it
with different data every time that method is called)
<> localPsfPixels -- localPsfPixels_allocated is set
allocated in ModelObject::AddPSFVector (copied from input pointer)
immediately passed to: psfInterpolator = new PsfInterpolator_bicubic(localPsfPixels, nPSFColumns, nPSFRows)
not used elsewhere
<> dataVector [external]
NOT allocated OR freed by ModelObject
pointer passed in via ModelObject::AddImageDataVector
<> weightVector
This has *two* modes:
1. Internally generated (& freed) -- weightVectorAllocated is set
2. Externally supplied (via --noise or --weight)
In ModelObject::AddErrorVector, weightVector is assigned to point
to the input error-image vector --> NOT freed by ModelObject
<> standardWeightVector -- standardWeightVectorAllocated is set
allocated in ModelObject::GetWeightImageVector, not used elsewhere
RETURNED by ModelObject::GetWeightImageVector
<> maskVector
This has *two* modes
1. Internally generated (& freed) -- maskVectorAllocated is set
In ModelObject::FinalSetupForFitting
2. Externally supplied (via --mask)
In ModelObject::AddMaskVector, maskVector is assigned to point
to the input mask-image vector --> NOT freed by ModelObject
<> residualVector -- residualVectorAllocated is set
allocated in ModelObject::GetResidualImageVector
RETURNED by ModelObject::GetResidualImageVector
<> deviatesVector -- deviatesVectorAllocated is set
allocated in ModelObject::ChiSquared
not used elsewhere (though it is passed to mp_enorm)
extraCashTermsVector
fblockStartFlags
bootstrapIndices
maskVectorAllocated
standardWeightVectorAllocated
residualVectorAllocated
outputModelVectorAllocated
deviatesVectorAllocated
extraCashTermsVectorAllocated
localPsfPixels_allocated
fblockStartFlags_allocated
PROBLEM: ModelObject::GetModelImageVector returns a ponter to either
modelVector or (if PSF convolution was done) outputModelVector.
Currently, imfit_main and makeimage_main merely pass these pointers
to SaveVectorAsImage and do nothing else, so neither is freed
outside of ModelObject.
modelVector is freed when ModelObject is destroyed
-- this will be bad for Python wrapper if user wants to
actually keep the model image (e.g, in numpy form)!
-- OK, Andre's code (ModelObjectWrapper.getModelImage,
used by me in imfit_lib.pyx) copies the data to an empty numpy array
outputModelVector is freed when ModelObject is destroyed
dataVector = modelVector = weightVector = standardWeightVector = NULL;
residualVector = maskVector = deviatesVector = NULL;
outputModelVector = extraCashTermsVector = NULL;
bootstrapIndices = NULL;
fblockStartFlags = NULL;
localPsfPixels = nullptr;
psfInterpolator = nullptr;
psfInterpolator_allocated = false;
ModelObject modifications:
Add PSF:
If pre-existing PSF and Convolver:
deallocate PSF and Convolver
If model-image size previously set:
trigger image-resizing/re-allocation
Same problem as for ModelObject: how to handle variable order of
supplying data images/model-image spec and PSF?
-- internal flags
data/model image size specified?
PSF size specified?
ModelObject::FinalSetupForFitting
Create a default all-pixels-valid mask if no mask already exists
Identify currently unmasked data pixels which have non-finite values and
add those pixels to the mask
Generate weight vector from data-based Gaussian errors, if using chi^2 + data errors
and no external error map was supplied
Generate extra terms vector from data for modified Cash statistic, if using latter
If an external error map was supplied, identify currently unmasked *error* pixels
which have non-finite values and add those pixels to the mask
Apply mask to weight vector (i.e., set weight -> 0 for masked pixels)
VetDataVector() [look for non-masked bad data values]
Final check that nValidDataVals >= 1
Python equivalents to SetupModelObject?
object.SetupForFitting( dataImage, maskImage, errorImage, psfImage, ... )
[or "AddDataForFitting?"]
.SetPSF( psfImage )
-- adds new PSF image (may be first, or replacement for previous)
.SetupForModelImage(nCols, nRows, psfImage, ...)
.ComputeModelImage( usePSF=True/False )
-- i.e., might be convenient to allow use to easily turn on/off
PSF convolution for testing/comparison purposes
.SetFitStatistic( ... )
-- specifies which fitting statistic to compute
-- may trigger GenerateExtraCashTerms(); allocation/deallocation
of extraCashTermsVector
** BUGS FIXED
[X] Fix imfit, makeimage, and imfit-mcmc so they error out if user supplies
oversampled PSF(s) but *not* oversampling regions
[X] Imfit on MBPro 2019 is very slow in oversampled-PSF tests (45s for fit
vs 3s on MBPro 2017)
-- true for all 3 tests (2 with N-M, 1 with L-M)
-- progression of actual recorded chi^2 values is identical
This is probably due to the problem of using more threads than actual cores
(via "hyperthreading") when doing FFT
[X] Fix unittest_add_functions.t.h -- testAddFunctionsToModel_optionalParams
-- use an extra-params function properly
[X] Fix image-comparison regression tests
OK, the weird case is biggertest.fits, which has two (and only two) pixels
which differ by ~ 0.0167 between the Mac and Linux versions; all other
pixels appear to be identical. The discrepant pixels are at (x,y) = (31,23)
and (31,39)
The first function-block has center at 31,31; the two discrepant pixels
are at y = 31 +/- 8
The difference shows up in the first Sersic function of the first function-block:
the mac version has I = 10.0, while the Linux version has I = 10.0167
OK, what seems to be happening is this:
1. The image created on the Linux VM is fine.
2. *Sometimes*, when the image is copied from the VM to the Mac side,
one or more pixels (minor-axis only, for some reason) with values of 10.0
get converted to 10.016664.
All *other* files agree to w/in 1e-13
[X] Update compare_fits_files.py to use numpy.allclose
[X] Update to use np.allclose( rtol=0, atol=1e-12)
[X] Test atol=1e-12 on existing Mac images
[X] write Python or shell script to do image comparisons from
makeimage regression tests
[X] Convert do_makeimage_tests to use new function
[X] Test atol=1e-12 on Linux images (on Mac)
[X] Move osx/*.fits images up one level
[X] Rewrite do_makeimage_tests to skip the OS testing and subdirectory use
[X] Add do_makeimage_tests to travis.yml
[X] PointSource component is not generated correctly in oversampled regions
-- xx July 20189 email from Iskren Georgiev
-- Problem was that oversampling from -X to +X is handled by calling
functions' GetValue() methods with (e.g.) x0 = -X to +X in 1/oversamplingScale increments
(e.g., -10.0, -9.8, -9.6, ...). This is correct for all functions *except*
PointSource, where -- in the case of an oversampled PSF -- we really should
be calling its GetValue() methods from -(oversamplingScale*X) in one-pixel
increments (because overampled PSF is sampled at finer scale0>
[x] Fixed by adding internal oversamplingScale data member to PointSource;
inside GetValue(), the x_diff and y_diff values are now computed by
multiplying (x - x0) and (y - y0) by oversamplingScale. Also added
SetOversamplingScale method to change internal oversamplingScale value.
-- OversampledRegion::ComputeRegionAndDownsample calls SetOversamplingScale
method of PointSource object to ensure correct oversamplingScale is being
used.
[x] Added check with simple oversampled-region + PointSource to do_makeimage_tests
[X] X0,Y0 coordinates in *printed* bootstrap output are *not* corrected for image sections
-- 17 April 2019 email from Chris Pritchet
-- see "Bootstrap and MCMC output" in "Bugs Fixed" section below for how I fixed this in MCMC
-- bootstrap results saved to (optional) output file *is* correct
-- BootstrapErrors() has the code that prints summary statistics to screen;
BootstrapErrorsBase() has the (correct!) code for writing results to file
by calling theModel->PrintModelParamsHorizontalString(paramsVect)
[x] Update BootstrapErrors() to use a modified (corrected) copy of bestfitParams
[input] with image offsets applied.
[x] Add checking of bootstrap output to do_imfit_tests
[X] MCMC restarts with --append turn fixed parameters into free?
Iskren Georgiev reports that restarting an MCMC run with --append
seems to have set fixed X0,Y0 into free.
[ ] Set up trial run with --append and fixed parameter(s)
./imfit-mcmc tests/faintstar.fits -c tests/imfit-mcmc_reference/config_imfit_faintstar.dat --no-subsampling --seed=1 -o temptest/mcmc_test_short --append &> temptest/test_dump_mcmc1c
Use existing config_imfit_faintstar_fixed.dat
./imfit-mcmc tests/faintstar.fits -c config_imfit_faintstar_fixed.dat --no-subsampling --seed=1 -o temptest/mcmc_test_short --max-chain-length=10000
./imfit-mcmc tests/faintstar.fits -c config_imfit_faintstar_fixed.dat --no-subsampling --seed=1 -o temptest/mcmc_test_short --append --max-chain-length=15000
Can't reproduce this -- everything behaves the way it should...
CONCLUSION: No bug present.
[X] Add oversampling to computation of single-function images
This was a long-standing bug I noticed when inspecting the code for other reasons;
basically, the oversampling stage of image computation was not being done when
single-function images were being computed
[X] Create or copy oversampled test image (single-function) for reference
[X] Test output using makeimage:
compare main output model image with single-function image: should
be different (bcs we haven't fixed anything yet!)
[X] Modify ModelObject::GetSingleFunctionImage to include oversampling
[X] Run unit tests
[X] Compile makeimage and run regression tests
[X] Test output using makeimage:
compare main output model image with single-function image: should
be identical
[X] Add test to do_makeimage_tests
[X] modify very first oversampling test so it also spits out single-function image
./makeimage tests/makeimage_reference/config_makeimage_gauss-oversample.dat -o temptest/oversampled.fits --psf tests/psf_standard.fits --overpsf tests/psf_oversamp.fits --overpsf_scale 3 --overpsf_region 100:110,100:110 --output-functions=temptest/oversampled_single
[X] Add test comparing reference image with single-function image
./python/compare_fits_files.py --min-value=1.0e-7 temptest/oversampled_single_Gaussian.fits tests/${osname}/oversampled_orig.fits
[X] Add test using two-function config file to do_makeimage_tests
-- e.g., use ./python/compare_fits_files.py --compare-sum to make sure individual
single-function images sum to match main output
(1.5)[x] Bootstrap and MCMC output:
X0,Y0 values are *not* corrected to full-image coordinates if input image used
image subsections (e.g., for "imfit data.fits[200:400,100:300] ...", the saved
X0,Y0 values in the bootstrap output file will be subsection-relative, although
the "best-fit" values in the header will be correctly full-image relative.
-- BootstrapErrors() --> BootstrapErrorsBase()
-- ModelObject::PrintModelParams -- corrections are applied *if* parameterInfo
input parameter is non-NULL
-- Need to write "horizontal" version of ModelObject::PrintModelParams
-- skips printing parameter names, but does apply x,y offsets
FIXED: Rewriting of printout methods to be more consistent; storing parameter-limits
info (including X0,Y0 offset values) inside ModelObject instead of trying to
pass them around to the PrintModelParams functions.
[x] Locate where in code we currently *do* correct for this
[x] Write unit test for new PrintModelParams function
[x] Write code to pass test
[x] Implement fix in bootstrap code
[x] Implement fix in MCMC code
[x] Add setting and storage of mp_param-type info *inside* ModelObject
-- idea is to circumvent the awkward, error-prone issue of how to pass through
mp_param pointer-to-array (or vector of mp_par) to ModelObject printing methods
-- could be simplified version of mp_par, maybe even just holding X0_offset,
Y0_offset
-- mp_par data members actually referenced within model_object.cpp:
PrintModelParams: .offset
print_results.cpp
PrintModelParamsToStrings: .offset, .fixed, .limits [+ .size() for vector<mp_par>]
mcmc_main.cpp
PrintModelParamsHorizontalString: .offset
bootstrap_errors.cpp
[x] Add vector<SimpleParameterInfo> to ModelObject
[x] Add printing method to ModelObject which uses internal vector
[x] Test substitution of new printing method in unit tests
[x] testPrintParamsToString
[x] PrintModelParamsHorizontalString -- testPrintModelParamsHorizontalString_noOffset
[x] PrintModelParamsHorizontalString -- testPrintModelParamsHorizontalString_withOffset
[x] PrintModelParamsHorizontalString -- testPrintModelParamsHorizontalString_withOffset_2blocks
[x] Convert code in print_results.cpp which uses PrintModelParams to use
PrintModelParamsToStrings instead, and then eliminate PrintModelParams ?
[x] Update ModelOject1D Print*Params methods
[x] Update adding parameter info to ModelObject1D in profilefit_main.cpp
[x] Update other parameter-printing code in profilefit
[X] PSF oversampling:
Small deviations in output image were present when generated using current working
version of makeimage (pre-1.5) with internal OversampledRegion vector vs when generated with
v1.4 of makeimage. (Also present when generated using makemultimages.)
FIXED: Problem was that oversampling was *not being done*, because the ModelObject
instance wasn't getting told in SetupModelObject() about the oversamplng, because we
weren't setting the correct variables in the MakeimageOptions object in main
(i.e., psfOversamplingScale and oversampleRegionSet [which are in base class] --
note that the former is distinct from the vector version psfOversamplingScales).
Fixed by explicitly setting options->oversampleRegionSet and
options->psfOversamplingScale before calling SetupModelObject().
[X] Convolution of an image with substantial region of negative pixels produces
weird ringing and positive pixels in negative region.