Skip to content

Some better error messages #189

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
opfromthestart wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Some better error messages #189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d945d7a
Some better error messages
opfromthestart Jan 31, 2023
264f38c
Slightly more clear
opfromthestart Jan 31, 2023
996d5c6
Errors to clarify current batch size limitations.
opfromthestart Jan 31, 2023
71da871
More info on input shape mismatch
opfromthestart Feb 11, 2023
632a2bc
Merge branch 'spearow:master' into better-error
opfromthestart Feb 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions juice/src/layer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,10 +545,22 @@ impl<B: IBackend> Layer<B> {
// reshape input tensor to the reshaped shape
let old_shape = self.input_blobs_data[input_i].read().unwrap().desc().clone();
if old_shape.size() != reshaped_shape.size() {
panic!(
"Input Shape Mismatch\nExpected {:?}\nActual {:?}",
reshaped_shape, old_shape
eprintln!(
"Input Shape Mismatch at layer {}\nLayer has input shape {:?}\nInput given has shape {:?}",
self.name, reshaped_shape, old_shape
);
if reshaped_shape[1..] == old_shape[1..] {

@drahnr drahnr Feb 1, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it'd make more sense to add an assert per dimension, and just fail with the first.

for dim in 0..5 {
     let old = old_shape[dim];
     let reshaped = reshaped__shape[dim];
     assert_eq!(old, reshpaed, "Dimension {dim} mismatches {old} != {reshaped}"); 
}
unreachable!("If the total size is not equal, one dimension mismatch must exist. qed");

@opfromthestart opfromthestart Feb 1, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I did it this way to catch some errors with mismatched batch sizes to make it a bit more clear.

@drahnr drahnr Feb 2, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it should always show the full dimensions as well. That way it stays clear where it could originate from your code. These errors can arise from rather deep inside the code.

@opfromthestart opfromthestart Feb 11, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe that my recent commit should have done this.

eprintln!("This may be a batch size error");
eprintln!("Expected batch size {} but got batch size {}.", reshaped_shape[0], old_shape[0]);
}
else if reshaped_shape[1..] == old_shape {
eprintln!("This may be a batch size error");
eprintln!("Expected batch size {} but got batch size {}.", reshaped_shape[0], 1);
}
if reshaped_shape == old_shape[1..] {
eprintln!("You may have forgotten to specify a batch size in your model input.");
}
panic!();
}
self.input_blobs_data[input_i]
.write()
Expand Down
3 changes: 3 additions & 0 deletions juice/src/layers/common/linear.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ impl<B: IBackend + LayerOps<f32>> ILayer<B> for Linear {
output_data: &mut Vec<ArcLock<SharedTensor<f32>>>,
output_gradient: &mut Vec<ArcLock<SharedTensor<f32>>>,
) {
if input_data.len() == 0 {
panic!("Linear layer expected input, but none was given.");
}
let input = input_data[0].read().unwrap();
let batch_size = input.desc()[0];
// reshape top
Expand Down
3 changes: 3 additions & 0 deletions juice/src/layers/container/sequential.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,9 @@ impl<B: IBackend + LayerOps<f32> + 'static> ILayer<B> for Sequential<B> {
output_data: &mut [ArcLock<SharedTensor<f32>>],
) {
for layer in &self.layers {
if layer.borrow().input_blob_names.len() < input_data.len() {
panic!("Layer {} expected {} input(s) but got {}.", layer.borrow().name, layer.borrow().input_blob_names.len(), input_data.len());
}

@drahnr drahnr Feb 1, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

馃憤

for (i, (input, input_name)) in input_data.iter().zip(self.input_tensor_names.iter()).enumerate() {
if &layer.borrow().input_blob_names[i] == input_name {
layer.borrow_mut().input_blobs_data[i] = input.clone();
Expand Down