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.

Sign up for GitHub

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: Driver --batch option sets Window Dimensions. #3770

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
+27 −9
Open

Fix: Driver --batch option sets Window Dimensions. #3770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
44b893b
Exception for replacing non-zero index dynamic-dimension by Batch
lakhinderwalia Jan 19, 2025
46b5fe8
print raw rate, without the batch multiplier
lakhinderwalia Jan 21, 2025
5966118
handle a case for default_dyn_dim_value option
lakhinderwalia Jan 23, 2025
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
5 changes: 3 additions & 2 deletions src/onnx/include/migraphx/onnx/onnx_parser.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* The MIT License (MIT)
*
* Copyright (c) 2015-2024 Advanced Micro Devices, Inc. All rights reserved.
* Copyright (c) 2015-2025 Advanced Micro Devices, Inc. All rights reserved.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -97,6 +97,7 @@ struct onnx_parser
std::unordered_map<std::string, instruction_ref> instructions;
program prog = program();
shape::dynamic_dimension default_dyn_dim_value = {1, 1};
size_t default_dim_value = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably default to 1.

std::unordered_map<std::string, std::vector<std::size_t>> map_input_dims;
std::unordered_map<std::string, shape::dynamic_dimension> dim_params;
std::unordered_map<std::string, std::vector<shape::dynamic_dimension>> map_dyn_input_dims;
Expand All @@ -121,7 +122,7 @@ struct onnx_parser
parse_graph(module* mod, const onnx::GraphProto& graph, bool inlining = false);
literal parse_value(const onnx::AttributeProto& attr) const;
literal parse_tensor(const onnx::TensorProto& t) const;
shape parse_type(const onnx::TypeProto& t) const;
shape parse_type(const std::string& name, const onnx::TypeProto& t) const;
shape parse_type(const onnx::TypeProto& t, const std::vector<std::size_t>& input_dims) const;
};

Expand Down
9 changes: 5 additions & 4 deletions src/onnx/onnx.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* The MIT License (MIT)
*
* Copyright (c) 2015-2024 Advanced Micro Devices, Inc. All rights reserved.
* Copyright (c) 2015-2025 Advanced Micro Devices, Inc. All rights reserved.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
Expand Down Expand Up @@ -45,8 +45,9 @@ program parse_onnx_from(const onnx_options& options, Ts&&... xs)
parser.map_input_dims = options.map_input_dims;
parser.dim_params = options.dim_params;
parser.map_dyn_input_dims = options.map_dyn_input_dims;
auto dim_val = options.default_dim_value;
if(dim_val != 0)
parser.default_dim_value = options.default_dim_value;

if(parser.default_dim_value)
{
if(options.default_dyn_dim_value != shape::dynamic_dimension{1, 1})
{
Expand All @@ -55,7 +56,7 @@ program parse_onnx_from(const onnx_options& options, Ts&&... xs)
}
else
{
parser.default_dyn_dim_value = {dim_val, dim_val};
parser.default_dyn_dim_value = {parser.default_dim_value, parser.default_dim_value};
}
}
else
Expand Down
11 changes: 9 additions & 2 deletions src/onnx/onnx_parser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@
}
else
{
s = parser.parse_type(input.type());
s = parser.parse_type(name, input.type());
}
mod_insts[name] = mod->add_parameter(name, s);
}
Expand Down Expand Up @@ -617,12 +617,13 @@
MIGRAPHX_THROW("PARSE_TENSOR: Invalid tensor type");
}

shape onnx_parser::parse_type(const onnx::TypeProto& t) const
shape onnx_parser::parse_type(const std::string& name, const onnx::TypeProto& t) const
{
shape::type_t shape_type = get_type(t.tensor_type().elem_type());

std::vector<shape::dynamic_dimension> dynamic_dims;
auto&& tensor_dims = t.tensor_type().shape().dim();
size_t idx = 0;
std::transform(tensor_dims.begin(),
tensor_dims.end(),
std::back_inserter(dynamic_dims),
Expand All @@ -632,11 +633,13 @@
const auto& dim_param = d.dim_param();
if(contains(dim_params, dim_param))
{
idx++;
return dim_params.at(dim_param);
}
}
if(d.has_dim_value())
{
idx++;
if(static_cast<int>(d.dim_value()) <= 0)
{
return default_dyn_dim_value;
Expand All @@ -646,6 +649,10 @@
}
else
{
if(idx && default_dim_value)

Check warning on line 652 in src/onnx/onnx_parser.cpp

View workflow job for this annotation

GitHub Actions / cppcheck 

style: Use 'and' instead of && [UseNamedLogicOperator]

Check warning on line 652 in src/onnx/onnx_parser.cpp

View workflow job for this annotation

GitHub Actions / tidy 

'&&' is a traditional token spelling, consider using an alternative token 'and' for consistency [readability-operators-representation,-warnings-as-errors]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why check default_dim_value and idx? If default_dim_value is 0 that would also be an error since it's would be setting a dimenion to zero?

MIGRAPHX_THROW("Batch inserted at index " + std::to_string(idx) +

Check warning on line 653 in src/onnx/onnx_parser.cpp

View check run for this annotation

Codecov / codecov/patch

src/onnx/onnx_parser.cpp#L653 

Added line #L653 was not covered by tests
" of " + name);
idx++;
return default_dyn_dim_value;
}
});
Expand Down
11 changes: 10 additions & 1 deletion src/program.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1008,7 +1008,16 @@
os << std::endl;

os << "Batch size: " << batch << std::endl;
os << "Rate: " << rate * batch << " inferences/sec" << std::endl;
if(batch > 1)
{
os << "Rate: " << rate * batch << " inferences/sec (with a Batch multiplier = " << batch

Check warning on line 1013 in src/program.cpp

View check run for this annotation

Codecov / codecov/patch

src/program.cpp#L1013 

Added line #L1013 was not covered by tests
<< ")" << std::endl;
os << "Raw Rate: " << rate << " inferences/sec" << std::endl;
}
else
{
os << "Rate: " << rate << " inferences/sec" << std::endl;
}
os << "Total time: " << total_time << "ms ";
os << "(Min: " << min_time << "ms, ";
os << "Max: " << max_time << "ms, ";
Expand Down
Loading