[GitHub] [madlib] njayaram2 commented on a change in pull request #398: Updated the code, state_size was pointing to the wrong value

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org


With regards,
Apache Git Services