Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds error handling to the load_tensors function to gracefully handle missing tensors during deserialization. Instead of failing when a tensor is not found in the archive, the function now catches exceptions and continues loading remaining tensors.
Changes:
- Wrapped
archive.read()call in a try-catch block to handle c10::Error exceptions - Added logic to skip missing tensors silently during deserialization
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| archive.read(pair.first, pair.second); | ||
| } catch (const c10::Error& e) { | ||
| // skip missing tensors |
There was a problem hiding this comment.
The catch block catches all c10::Error exceptions, which includes many different types of errors beyond just missing tensors. This could mask serious issues like corrupted archives, I/O errors, or type mismatches. Consider checking the error type or message to only skip errors specifically related to missing tensors, and re-throw other types of errors that indicate real problems.
| // skip missing tensors | |
| // Only skip errors that clearly indicate a missing tensor/key. | |
| const std::string msg = e.what(); | |
| if (msg.find("archive entry not found") == std::string::npos && | |
| msg.find("Could not find key") == std::string::npos && | |
| msg.find("Could not find tensor") == std::string::npos) { | |
| // Unexpected error: rethrow so it can be handled by the caller. | |
| throw; | |
| } | |
| // Known missing-tensor error: skip this entry. |
| try { | ||
| archive.read(pair.first, pair.second); | ||
| } catch (const c10::Error& e) { | ||
| // skip missing tensors | ||
| } |
There was a problem hiding this comment.
The load_tensors function's behavior has fundamentally changed - it now silently skips missing tensors instead of failing. This is a significant change to the API contract that should be documented. Consider adding a comment to the function header in serialize.hpp explaining that missing tensors will be silently skipped, or alternatively, consider whether the caller should have control over this behavior through an optional parameter.
| } catch (const c10::Error& e) { | ||
| // skip missing tensors | ||
| } |
There was a problem hiding this comment.
The catch block silently suppresses all errors without any logging or feedback. This makes debugging difficult when tensors fail to load unexpectedly. Consider logging at least a warning with the tensor name and error details to help diagnose issues. For example, you could add a log statement indicating which tensor was not found in the archive.
|
🎉 Released v1.2.9! What's Changed
Full Changelog: v1.2.8...v1.2.9 |
1 participant
No description provided.