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

Fix rocAL Tensor build issues PR 4 - Pipeline and Pybind #28

Merged

Conversation

fiona-gladwin
Copy link
Collaborator

  • Re introduced the copy tensor API in master graph as it gives performance in tensor trainings
  • Re-introduce some external API to get width and height from master graph as suggested
  • Introduce API and changes required to get original ROI from tensor (Similar to TOT changes)
  • Fix build issues with pybind

@@ -50,6 +50,25 @@ THE SOFTWARE.
#define MAX_NUM_ANCHORS 8732 // Num of bbox achors used in SSD training
#define MAX_MASK_BUFFER 10000

#if ENABLE_SIMD
Copy link
Collaborator

Choose a reason for hiding this comment

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

this seems to be missing in tot MIVIsionX. Why is this needed here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This portion was a part of master_graph.cpp.
We have moved the masks as constants into the master_graph.h hence we have moved the include part also from the cpp file into master_graph.h file


_convert_time.start();
// Copies to the output context given by the user
auto dims = _output_tensor_list[0]->info().dims();
Copy link
Collaborator

Choose a reason for hiding this comment

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

please use another pointer to point to output_tensor_list[0]->info() and use that everywhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

const size_t c = dims[3];
const size_t h = dims[1];
const size_t w = dims[2];
const size_t single_output_image_size = _output_tensor_list[0]->info().data_size();
Copy link
Collaborator

Choose a reason for hiding this comment

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

rename to tensor specific name

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@@ -112,6 +112,8 @@ void TensorInfo::reset_tensor_roi_buffers() {
size_t roi_size = (_layout == RocalTensorlayout::NFCHW || _layout == RocalTensorlayout::NFHWC) ? _dims[0] * _dims[1] : _batch_size; // For Sequences pre allocating the ROI to N * F to replicate in OpenVX extensions
allocate_host_or_pinned_mem((void **)&_roi_buf, roi_size * 4 * sizeof(unsigned), _mem_type);
}
if(!_original_roi_height->size()) _original_roi_height = std::make_shared<std::vector<uint32_t>>(_batch_size);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is changed from struct roi to width and height

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed these _original_roi_width and _original_roi_height as they are not used anymore

numpy_array = py::array(py::buffer_info(
((unsigned char *)(output_tensor.buffer())) + idx * (info.strides()[0]/sizeof(float)),
((unsigned char *)(output_tensor.buffer())) + idx * (output_tensor.strides()[0]/sizeof(float)),
Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this dividing by sizeof(float) for UINT8

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed

Copy link
Collaborator

@rrawther rrawther left a comment

Choose a reason for hiding this comment

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

please address review comments

Avoid the conflicting function name between tensor and image files
@fiona-gladwin
Copy link
Collaborator Author

please address review comments

@rrawther and @LakshmiKumar23 I have addressed all review comments. Please check.

@LakshmiKumar23 LakshmiKumar23 merged commit ac4f552 into LakshmiKumar23:lk/rocalTensor Jul 17, 2023
@fiona-gladwin fiona-gladwin deleted the PR_tensor_w4 branch August 7, 2023 18:28
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.

3 participants