Error handling struct #76
Conversation
This reverts commit 1844108.
There was a problem hiding this comment.
After a few fixes, I still get a segmentation fault in
ccl_test_cls.c, line 109
(CCL_ClTracer *tr_wl_1=ccl_cl_tracer_lensing_simple_new(cosmo,nz,zarr_1,pzarr_1,&status);),
which I haven't been able to track down yet; I will look into this tomorrow.
In addition to this problem, @philbull, @damonge, can you comment on what changes to the python wrapper routines will be required before merging in this pull request?
|
I've reviewed this and pushed a bunch of changes.
|
|
@elikrause: The call signatures of all the functions have changed, so we should review the Python wrapper to make sure nothing has broken. I've written some unit tests that should help with this (currently in the pywrapper branch). Python doesn't really have input/output function arguments like C does, so we'll have to decide how to deal with that. swig will probably add the status variable to the end of the list of return values, but it'd be a bit inconvenient to return a second variable every time the user calls any function. So, I suggest writing a "decorator" that we apply to each function to do the status handling in a more Pythonic way. I'll investigate this, but I'm not sure if I can have it finished by Wednesday. |
|
yes @elikrause, some of this was because there are some flags that are used in testing "if" statements etc, where the pointer class was not appropriate - but I'll also readily admit that it was in a desire (but possibly a failing) to be consistent through the module. |
…hrough the status pointer are not handled yet, though.
|
OK, I went through the Python wrapper and managed to make most of the changes that were needed (see the most recent commit). I'm getting a segfault in ccl_massfunc for some reason, so I'll debug that next. |
* Fixed segfault caused by unallocated struct member in SigmaR_pars * Added status checking to new background functions in master
|
@reneehlozek why are the files like class/include/hermite3_interpolation_csource.h removed? |
| @@ -14,6 +15,9 @@ void ccl_check_status(ccl_cosmology *cosmo){ | |||
| case CCL_ERROR_SPLINE: // spline allocation error, always terminate | |||
| fprintf(stderr,"%s",cosmo->status_message); | |||
There was a problem hiding this comment.
The status_message will no longer necessarily match the integer that you pass in now. In fact if we're going for thread safety the status_message violates that just as the status integer did before. I think the right thing to do is not look anything up in the cosmo struct but instead to map the possible values of the status integer to messages.
There was a problem hiding this comment.
I agree that this may scramble the status_message in cosmo, so in the current version status_message may not be helpful for debugging and one should rely only on the error code...
|
What is the plan if status!=0 on entry to a function? Just fall through like cfitsio does? |
|
|
||
| # Check for known error status | ||
| if status in core.error_types.keys(): | ||
| raise RuntimeError("Error %d: %s" % (status, core.error_types[status])) |
There was a problem hiding this comment.
I'd suggest creating a new error class derived from StandardError that can take the status value as a parameter, so that it is easier to catch certain types of error.
There was a problem hiding this comment.
Yes, this will happen eventually, but isn't critical for this release.
|
I'm merging the latest from master into this branch. Hang fire while I fix a few errors... |
* Fix conflicts with status handling in new massfunction functions * Fix Python wrapper given changes in massfunction
* master: Update ccl_sample_run.c small change in user_pz_probability Now passing status to halo_bias Default config changed to Tinker10 Adding halo bias in the CCL example updated compilation command upated REAME with lsst_spec example updated lsst_spec example fix ccl_specs_free_photoz_info fix ccl_specs_free_photoz_info Other minor fixed Fixed lines that were being cut-off Changes in the README to update the compilation of the CCL example # Conflicts: # src/ccl_massfunc.c
I updated the status variables to be pointers and have passed them throughout the code.
This is propagated through the src and test codes.
In the src code, additional non-pointer status variables where created when needed for "if" statements.
[eg. look for pkstatus in src/ccl_power.c]