njayaram2 commented on a change in pull request #398: Updated the code,
state_size was pointing to the wrong value
URL: https://github.com/apache/madlib/pull/398#discussion_r288304075
##########
File path: src/ports/postgres/modules/convex/mlp_igd.py_in
##########
@@ -334,7 +334,7 @@ def mlp(schema_madlib, source_table, output_table,
independent_varname,
"""
it.update(train_sql)
if it_args['state_size'] == -1:
- it_args['state_size'] = it.get_state_size()
+ it.kwargs['state_size'] = it.get_state_size()
Review comment:
This if condition will be True for every iteration since
`it_args['state_size']` is not updated after the first time it goes into the if
statement. So we will end up running the query in `it.get_state_size()`
multiple times which is unnecessary (the `state_size` will not change). Can we
update `it_args['state_size']` too inside this if statement (or something
similar which will preclude this if statement from evaluating to True after the
first time)? Something simple would be, but feel free to do anything else that
might be cleaner/efficient:
```
if it_args['state_size'] == -1:
it.kwargs['state_size'] = it.get_state_size()
it_args['state_size'] = it.kwargs['state_size']
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
