GNU 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 | Percent Complete: | 60% |
Assigned to: | None | Open/Closed: | Open |
Effort: | 0.00 |
Sun 29 Jan 2023 01:13:59 PM UTC, comment #4: |
Mohammad Akhlaghi <makhlaghi>![]() |
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:
Here, the 8 bit library code(`lib_code`) indicates which library
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:
|
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.
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.
Please share your thoughts on these proposals here... |
Mohammad Akhlaghi <makhlaghi>![]() |
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>![]() |
No files currently attached
Depends on the following items: None found
Items that depend on this one: None found
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.
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).