taskGNU Astronomy Utilities - Tasks: task #16263, Errors in the library: remove all...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

task #16263: Errors in the library: remove all exit() calls

Submitter:  Mohammad Akhlaghi <makhlaghi>
Submitted:  Sat 17 Sep 2022 08:42:05 PM UTC
   
 
Should Start On:  Fri 16 Sep 2022 11:00:00 PM UTC Should be Finished on:  Fri 16 Sep 2022 11:00:00 PM UTC
Category:  Libraries Priority:  5 - Normal
Item Group:  Enhancement Status:  In Progress
Privacy:  Public Assigned to:  None
Percent Complete:  60% Open/Closed:  Open
Effort:  0.00

Thu 27 Apr 2023 02:25:23 AM UTC, comment #5: 

Hello, I did a commit with the changes made to functions in array.c. I would like to know your feedback on this so that I can share the rest of the work.
commit link: https://github.com/fathmamehnoor/gnuastro-dev/tree/add-error-reporting

Fathma Mehnoor <fathmamehnoor>
Sun 29 Jan 2023 01:13:59 PM UTC, comment #4: 

After Jash's first implementation, I made some further modifications and the latest work on this task is now in the 'error' branch of Gnuastro on Codeberg.

We have made some good progress, but some work still remains to fully integrate this into the library (follow from the commit messages).

Mohammad Akhlaghi <makhlaghi>
Group administrator
Wed 09 Nov 2022 12:12:48 PM UTC, comment #3: 

I've been working on the new error.h module for the library. After many discussions, we decided to go with the gal_error_t structure to hold a list of all the errors encountered as suggested by Mohammad. However, a few additions have been made:

typedef struct gal_error_t
{
  uint8_t code;              /* Code of the problem wrt to each library.*/
  uint8_t lib_code;          /* Library which created the error.        */
  uint8_t is_warning;        /* Defines if the error is only a warning. */
  char *back_msg;            /* Detailed message of backend (library)   */
  char *front_msg;           /* Detailed message of front end (caller). */
  struct gal_error_t *next;  /* Next error message.                     */
} gal_error_t;


Here, the 8 bit library code(`lib_code`) indicates which library
generated the error. All libraries have their own unique
code defined in alphabetical order inside an enum.

8 bit flag indicating if the error is a breaking error or just a warning(`is_warning`).

The 8 bit error typecode(`code`) indicates the type of error within that library. Each library has its own error types defined as 32-bit macros. These macros are constructed to hold the value of all three above-mentioned attributes.

I've also added some basic functions for error handling and parsing of the gal_error_t structure. The cosmology module has been changed to now use this new error module.

Here is the commit with the work done till now:
https://github.com/Jash-Shah/gnuastro-jash/commit/f8fd0a8ccdc863ac0b430da2ce3c28d0be86e13e

Jash Shah <jash_shah>
Tue 20 Sep 2022 08:28:36 AM UTC, comment #2: 

The structure of the error type seems great! Also, the ability to have the errors accumulated will be very useful, especially in cases where multiple errors from a function call stack need to be analysed.

Thinking from the user's perspective, the first implementation seems the best approach, because of:

1. Not having to carry around an extra error structure with every `gal_data_t` instance.

2. Having a uniform way of checking for errors across all modules. For users, this becomes easy as they can form a singular habit of checking if the error structure they passed returned an error or not. There can be one set rule of error checking on the user side instead of having to check which rule out of the two applies for their usecase.

3. The implementation and documentation will also be easier and we can have reapplicable set and get methods for the same.

I can start trying out the implementation of the same for the FITS image read and write functions.

Jash Shah <jash_shah>
Mon 19 Sep 2022 04:32:57 PM UTC, comment #1: 

After some discussions with Pedram and Jash on #gnuastro-dev:openastronomy.org we have made some general progress in the concept.

In summary, currently, the error messages in Gnuastro's library are very elaborate and customized for each function, and each input, including even solutions based on where they happen. This type of customized error messages are not possible with merely passing an integer. We can therefore have a structure like the following to have the best of both worlds: 1. generic integers to help the high-level caller decide what to do, and 2. a full string, with a detailed description of that particular situation, 3. a pointer to a 'next' element, to let the caller accumulate the messages if they need to.


typedef struct gal_error_t
{
  int code;                  /* Generic code of the problem.  */
  char *back_msg;            /* Detailed message of backend (library) */
  char *front_msg;           /* Detailed message of front end (caller). */
  struct gal_error_t *next;  /* Next error message.           */
} gal_error_t;


To be very generic (also looking from a high-level perspective), the caller may also like to add a message (for example saying that "we tried function XXX with parameter YYY, but it failed with the following error").

Each library can have its own set of codes to help the caller account for different types of errors without having to parse the message. But the messages are very useful for the user of the high-level program (like the Python modules).

We can have specialized functions to deal with this structure (like adding an item, printing them in a uniform format and etc.

We can implement this in the functions that currently call 'error()'

1. They can be given an extra 'gal_error_t **' pointer as an argument.
2. We can add 'gal_error_t' as an element of 'gal_data_t', so the functions that return a 'gal_data_t' can simply set that pointer within the returned dataset. For the functions that don't return 'gal_data_t' we can revert to case 1.

Please share your thoughts on these proposals here...

Mohammad Akhlaghi <makhlaghi>
Group administrator
Sat 17 Sep 2022 08:42:05 PM UTC, original submission:  

Currently, when the library functions hit a bad situation, they use the 'error(EXIT_FAILURE, ....)' function to inform the user and abort.

For Gnuastro's own programs (which until now where the main callers of the library), this was not a problem (and infact they are designed around this!). But for other callers (like pyGnuastro, this is problematic and will cause an exit from the whole Python environment!

The way that CFITSIO or WCSLIB deal with this is very good: all their functions have an 'int *status' argument for various types of reasons it can crash. After it returns, if 'status!=0', then there was an error.

To fix this problem, we should define a special 'enum' in each header for all the types of crashes that occur in that header file, and assign the status element as a macro there. The caller can then check status, and if it isn't 0, they can call a special function in all libraries (for example 'gal_fits_error_message' for the functions in 'fits.h') to get the complete message for that particular status code. In the case of `pyGnuastro`, to pass the message to the user (instead of aborting the whole Python program!).

This bug was reported by Faeze Bidjarchian, while we were testing pyGnuastro.

Mohammad Akhlaghi <makhlaghi>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by fathmamehnoor (Posted a comment)
  • -email is unavailable- added by jash_shah (Posted a comment)
  • -email is unavailable- added by makhlaghi (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-01-29 makhlaghi StatusNone In Progress
        Percent Complete0% 60%
    2022-09-19 makhlaghi SummaryRemove all exit() calls within the library Errors in the library: remove all exit() calls

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code