acsresearch / interlab

MIT License
17 stars 2 forks source link

Should the context store the Exception type? #19

Closed jas-ho closed 1 year ago

jas-ho commented 1 year ago

currently, the context stores a generic context.error._type = 'error' when an error is raised within a context image

this was a bit surprising to me since I expected error._type to contain some information about the type of exception which was raised. Should this be the case?

jas-ho commented 1 year ago

Note that the current implementation seems a bit inconsistent to me since for most other objects _type actually references the class of the object

e.g. for dataclass inputs image

jas-ho commented 1 year ago

same for Tag and Context

jas-ho commented 1 year ago

other possible inconsistencies:

spirali commented 1 year ago

Type "error" is a bit inheritance from older version. The reason was that I wanted to include textual form of backtrace to errors and some additional debug info (not implemented yet). These things are not actual attributes of Exception. But I do not have a strong opinion for this.

spirali commented 1 year ago

other possible inconsistencies:

* a  function decorated with `with_context` creates a context of `_type=Context`  (whereas `type(my_function) = function`)

* calls to LLMs create a context of `_type=Context`

This behavior is intended. @with_context is just a shortcut for creating context around a function call.

jas-ho commented 1 year ago

personally I think it will be long-term beneficial if the _type mirrors the Python type as closely as possible (definitely for exception types, maybe other cases too).

imagine e.g. a very long-running experiments with a bunch of different exceptions occurring. it could be very helpful to be able to filter down to just displaying specific exceptions.

currently, I see errors such as the one below where the _type field does not contain useful info (imo)

 ...
  "state": "error",
  "error": {
    "_type": "error",
    "name": "engine repeatedly returned a response without a valid JSON instance of type"
  },
  ...

thoughts on this @spirali @gavento?

spirali commented 1 year ago

I totally agree that error system is now insufficient (exact type of exception is definitely missing among other things). Generally I do not have nothing about storing exact type in the case exceptions, the only thing I want to somehow preserve is information that particular type inherits from the Exception class.

jas-ho commented 1 year ago

Solved by #47