-
Notifications
You must be signed in to change notification settings - Fork 101
Fixing size parameter handling in len-mechanism to handle function parameters with different size parameters #2138
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
base: master
Are you sure you want to change the base?
Changes from all commits
10bd251
ddf7097
2700665
bd858fd
287ad78
d2ee442
9488807
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -348,23 +348,31 @@ def filter_parameters(parameters, parameter_usage_options): | |||||||||
| parameters_to_use = [] | ||||||||||
|
|
||||||||||
| # Filter based on options | ||||||||||
| size_parameter = None | ||||||||||
| ivi_dance_size_parameter = None | ||||||||||
| len_size_parameter_names = set() | ||||||||||
| size_twist_parameter = None | ||||||||||
| # If we are being called looking for the ivi-dance, len or code param, we do not care about the size param so we do | ||||||||||
| # not call back into ourselves, to avoid infinite recursion | ||||||||||
| if parameter_usage_options not in [ParameterUsageOptions.IVI_DANCE_PARAMETER, ParameterUsageOptions.LEN_PARAMETER]: | ||||||||||
| # Find the size parameter - we are assuming there can only be one type, either from ivi-dance or len | ||||||||||
| size_parameter = find_size_parameter(filter_ivi_dance_parameters(parameters), parameters) | ||||||||||
| if size_parameter is None: | ||||||||||
| size_parameter = find_size_parameter(filter_len_parameters(parameters), parameters) | ||||||||||
| # Determine any size parameters that should be skipped based on the presence of ivi-dance or len-sized buffers. | ||||||||||
| # For ivi-dance, there is a single shared size parameter; for len, there may be multiple independent size parameters. | ||||||||||
| ivi_dance_size_parameter = find_size_parameter(filter_ivi_dance_parameters(parameters), parameters) | ||||||||||
| # Collect all size parameter names referenced by len-sized parameters | ||||||||||
| len_params = filter_len_parameters(parameters) | ||||||||||
| for param in len_params: | ||||||||||
| len_size_param = find_size_parameter(param, parameters) | ||||||||||
| if len_size_param is not None: | ||||||||||
| len_size_parameter_names.add(len_size_param['name']) | ||||||||||
|
Comment on lines
+361
to
+365
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would be better to push most of this logic into a |
||||||||||
| size_twist_parameter = find_size_parameter(filter_ivi_dance_twist_parameters(parameters), parameters, key='value_twist') | ||||||||||
| for x in parameters: | ||||||||||
| skip = False | ||||||||||
| if x['direction'] == 'out' and options_to_use['skip_output_parameters']: | ||||||||||
| skip = True | ||||||||||
| if x['direction'] == 'in' and options_to_use['skip_input_parameters']: | ||||||||||
| skip = True | ||||||||||
| if x == size_parameter and options_to_use['skip_size_parameter']: | ||||||||||
| if ivi_dance_size_parameter is not None and x == ivi_dance_size_parameter and options_to_use['skip_size_parameter']: | ||||||||||
| skip = True | ||||||||||
| if len_size_parameter_names and x['name'] in len_size_parameter_names and options_to_use['skip_size_parameter']: | ||||||||||
| skip = True | ||||||||||
| if size_twist_parameter is not None and x == size_twist_parameter and options_to_use['skip_size_parameter']: | ||||||||||
| skip = True | ||||||||||
|
|
@@ -443,18 +451,15 @@ def filter_ivi_dance_twist_parameters(parameters): | |||||||||
| def filter_len_parameters(parameters): | ||||||||||
| '''Returns the len parameters of a session method if there are any. These are the parameters whose size is determined at runtime using the value of a different parameter. | ||||||||||
|
|
||||||||||
| asserts all parameters that use len reference the same parameter | ||||||||||
| Note: Multiple len parameters may reference different size parameters. | ||||||||||
| Args: | ||||||||||
| parameters: parameters to be checked | ||||||||||
|
|
||||||||||
| Return: | ||||||||||
| None if no len parameter found | ||||||||||
| Parameters dict if one is found | ||||||||||
| Empty list if no len parameter found | ||||||||||
| List of parameters if any are found | ||||||||||
|
Comment on lines
+459
to
+460
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
| ''' | ||||||||||
| params = filter_parameters(parameters, ParameterUsageOptions.LEN_PARAMETER) | ||||||||||
| if len(params) > 0: | ||||||||||
| size_param = params[0]['size']['value'] | ||||||||||
| assert all(x['size']['value'] == size_param for x in params) | ||||||||||
| return params | ||||||||||
|
|
||||||||||
|
|
||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,8 +11,8 @@ | |
| ivi_dance_parameters = helper.filter_ivi_dance_parameters(parameters) | ||
| ivi_dance_size_parameter = helper.find_size_parameter(ivi_dance_parameters, parameters) | ||
| len_parameters = helper.filter_len_parameters(parameters) | ||
| len_size_parameter = helper.find_size_parameter(len_parameters, parameters) | ||
| assert ivi_dance_size_parameter is None or len_size_parameter is None | ||
| len_size_parameters = helper.find_size_parameter(len_parameters, parameters) | ||
| assert ivi_dance_size_parameter is None or len_size_parameters is None | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So we're not supporting mixed usage, after all? |
||
|
|
||
| full_func_name = f['interpreter_name'] + method_template['method_python_name_suffix'] | ||
| c_func_name = config['c_function_prefix'] + f['name'] | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You deleted this comment and the if statement guarding population of len size parameters.
But I don't see any testing for mixed usage.