Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review use of exceptions #18

Open
BenBrock opened this issue Apr 12, 2023 · 1 comment
Open

Review use of exceptions #18

BenBrock opened this issue Apr 12, 2023 · 1 comment
Assignees

Comments

@BenBrock
Copy link
Collaborator

BenBrock commented Apr 12, 2023

Scott began a section on exceptions, which I have updated.

Scott also added some notes on errors in the GraphBLAS C API, which I have moved to below:

Notes on C API errors

Informational return values

  • SUCCESS: not necessary with exceptions
  • NO_VALUE: happens in extractElement() method when providing a transparent destination scalar and no stored value. Using grb::out_of_range exception. The justification for changing informational return with exception is the consistency with STL container at() method behaviour.

API errors

  • UNINITIALIZED_OBJECT: not necessary with RAII philosophy
  • NULL_POINTER: not necessary, no pointers used in C++ API
  • INVALID_VALUE:
    • used with incorrect enum values (not necessary in C++)
    • used if a dimension is defined as 0 (like in vector or matrix construction or resize, (do we need this?)
    • duplicate indices during build (do we need this?).
  • INVALID_INDEX: This is specifically for indices that are outside the defined dimensions (for setElement(), extractElement(), removeElement()). Does this overload with NO_VALUE?
  • DOMAIN_MISMATCH: not applicable in C++...if it compiles there is no mismatch.
  • OUTPUT_NOT_EMPTY: was used when trying to build with a container that is not empty....what should C++ do?
  • NOT_IMPLEMENTED: ???

Execution Errors

  • PANIC - do we need this?
  • OUT_OF_MEMORY: using std::bad_alloc... why not grb::bad_alloc? Because allocators not part of GraphBLAS spec.
  • INSUFFICIENT_SPACE: ???
  • INVALID_OBJECT: The requires a discussion of exception quarantees. I.e. what is the state of an output object when an exception is thrown during an operation it was called with? This is also complicated by non-blocking mode which is deferred for now.
  • INDEX_OUT_OF_BOUNDS: grb::out_of_range (This is overloaded use...for two different meanings)
  • EMPTY_OBJECT: (only called in reduce to scalar...does C++ API side step this)
@BenBrock BenBrock changed the title Write section on exceptions, normalize names of exceptions Review use of exceptions May 16, 2023
@BenBrock BenBrock assigned mcmillan03 and unassigned tgmattso May 16, 2023
@mcmillan03
Copy link
Member

Exceptions and Error Handling section is currently under Algorithms

  • this should be promoted to its own section under Intro or Basic Concepts.

Are all grb:: exceptions subclassed from std::exception?...or we could consider subclassing from std::logic_error and std::runtime_error classes instead.

  • std::logic_error subclass roughly corresponds to API errors
    - E.g., invalid args, domain, dimensions, out_of_range)
  • std::runtime_error subclass roughly corresponds to execution errors
    - E.g., panic, file_io, over/underflow, system_error
  • Should we consider a similar organization:
    - grb::api_errors and grb::execution_errors, OR
    - grb::logic_errors and grb::runtime_errors
  • From CppCoreGuidelines: "Deriving from std::exception gives the flexibility to catch the specific exception or handle generally through std::exception"
  • subclassing from grb::exception allows the user to separate grb specific exceptions from others
    - I am not sure this is a necessary consideration, and we should consider the alternative of logic_error/runtime_error subclassing instead.

We need to be sure to review spec to make sure all possible noexcept specs are specified.

  • Why not some of the operator() methods of the operators? because someone could conceivably write a scalar type that requires allocation.

All calls to Allocator::allocate may throw. Do we need to specify which exception(s)?

  • grb::matrix ctor
  • grb::vector ctor (this constructor is not documented the same as matrix constructor)
  • matrix::read_matrix(). Soon to be free function.

grb::out_of_range

  • matrix::at(),vector::at() Is there a precedent out_of_range is not intuitive name for no stored element, it seems more appropriate name for indices that are outside of range of the dimensions. Perhaps consider grb::invalid_location exception

grb::matrix_io::file_error,
grb::matrix_io::unsupported_file_format

  • should this be under matrix_io at all? Might prefer grb::file_io:: namespace

grb::invalid_argument - currently for dimension mismatch, Why not grb::invalid_dimensions

@mcmillan03 mcmillan03 moved this from Todo to In Progress in Path to GraphBLAS 1.0 Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

No branches or pull requests

3 participants