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

Add in exceptions and verbose errors #3611

Merged
merged 7 commits into from
Mar 28, 2022
Merged

Add in exceptions and verbose errors #3611

merged 7 commits into from
Mar 28, 2022

Conversation

muellerzr
Copy link
Contributor

Includes a variety of verbose errors for commonly situated problems. Due to the complexity of some issues, some of the logic in said errors is a bit much in order to provide a clear picture of what went wrong (such as the collate_error).

Open to removing any you're not comfortable with @jph00

@muellerzr muellerzr requested a review from jph00 as a code owner March 25, 2022 22:16
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@muellerzr
Copy link
Contributor Author

Aware there's a small bug with the tests in there I think. Will get to it in the next few days

@jph00
Copy link
Member

jph00 commented Mar 26, 2022

@muellerzr can you take a look at this CI failure? Might just be a case of needing to merge lastest fastai changes - not sure.

@muellerzr
Copy link
Contributor Author

@jph00 okay, that was an exploration and a half but we got callbacks working. One thing I did have to do though was move up where the exception classes were defined so they can be pulled into Callback.__call__. Since we already use show_doc for them all though, I think it would make sense to keep the documentation in the same spot

Comment on lines +58 to +59
except (CancelBatchException, CancelEpochException, CancelFitException, CancelStepException, CancelTrainException, CancelValidException): raise
except Exception as e:
Copy link
Contributor Author

@muellerzr muellerzr Mar 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also rewrite this as something like:

except Exception as e:
  if len(e.args) > 0: 
    e.args = [f'Exception occured in `{self.__class__.__name__}` when calling event `{event_name}`:\n\t{e.args[0]}']
  raise

However that could potentially lead to trouble down the road. (I could also do a huge isinstance too)

@jph00
Copy link
Member

jph00 commented Mar 28, 2022

Thanks @muellerzr !

@jph00 jph00 merged commit 5763041 into fastai:master Mar 28, 2022
@muellerzr muellerzr deleted the error-logging branch March 28, 2022 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants