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

Rocal classification PR #30

Conversation

SundarRajan28
Copy link

Changed required for classification training convergence

src_w_dims = _inputs[0]->info().get_roi_width_vec();
src_h_dims = _inputs[0]->info().get_roi_height_vec();
original_w_dims = _inputs[0]->info().get_original_roi_width_vec();
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could have an if condition to check if it is resize shorter, then we can assign original roi width and height to the src_w_dims and src_h_dims and then call the adjust_out_roi_size function to maintain uniformity

Also add a comment

Copy link
Author

Choose a reason for hiding this comment

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

Resolved in the latest commit. Will trigger a training to confirm convergence. Slightly doubtful whether std::lround in adjust_roi_size may cause difference in the resize dimensions.

host_memory_padding_jpeg2k=0, hybrid_huffman_threshold=1000000, memory_stats=False, normalized_anchor=True,
normalized_shape=True, output_type=types.RGB, preserve=False, seed=1, split_stages=False, use_chunk_allocator=False,
use_fast_idct=False, device=None, decode_size_policy=types.USER_GIVEN_SIZE_ORIG, max_decoded_width=1000, max_decoded_height=1000):

reader = Pipeline._current_pipeline._reader
#Reader -> Randon BBox Crop -> ImageDecoderSlice
#Random crop parameters taken from pytorch's RandomResizedCrop default function arguments
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it specifically used for classification?

Copy link
Author

Choose a reason for hiding this comment

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

Checked with swetha and found that its used only for classification

// Using original width and height instead of the decoded width and height for computing resize shorter dimensions
src_w_dims = _inputs[0]->info().get_original_roi_width_vec();
src_h_dims = _inputs[0]->info().get_original_roi_height_vec();
}
Copy link
Owner

Choose a reason for hiding this comment

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

For all modes we should calculate using original width and height ryt?
Can we change this to default?

Copy link
Author

Choose a reason for hiding this comment

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

Resolved in the latest commit

@@ -90,7 +90,7 @@ Decoder::Status FusedCropTJDecoder::decode(unsigned char *input_buffer, size_t i
max_decoded_width * planes,
max_decoded_height,
tjpf,
TJFLAG_FASTDCT, &x1_diff, &crop_width_diff,
TJFLAG_ACCURATEDCT, &x1_diff, &crop_width_diff,
Copy link
Owner

Choose a reason for hiding this comment

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

Are we not converging with TJFLAG_FASTDCT. Has that been checked?
Also was there any performance impact because of change from fast to accurate?

Copy link
Author

Choose a reason for hiding this comment

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

Will be triggering a training to confirm convergence with FastDCT and use FastDCT if its converging. With regards to performance, I've not observed any performance drop with AccurateDCT algorithm

@SundarRajan28 SundarRajan28 changed the base branch from PR_branch to develop February 2, 2023 14:27
@SundarRajan28 SundarRajan28 changed the base branch from develop to PR_branch February 2, 2023 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants