search

PathmindAI / nativerl

Train reinforcement learning agents using AnyLogic or Python-based simulations
Apache License 2.0
19 stars 4 forks source link

Add support for `+=` and `len()` on NativeRL Arrays #511

Closed slinlee closed 2 years ago

slinlee commented 2 years ago

This replaces #508.

@saudet Thanks for the pointer for the docs on overloading operators. I think I was able to add += . This is the next error this brings me to is, which I'm not sure how to fix:

  File "/app/work/pathmind_training/environments.py", line 285, in <listcomp>
    for i in range(self.num_reward_terms)
TypeError: 'nativerl.Array' object is not subscriptable

The code it's referring to is : https://github.com/SkymindIO/nativerl/blob/6f488187b0bba38fd68d0b6f1283ceabb1bfc1b2/nativerl/python/pathmind_training/environments.py#L279-L288

cc @maxpumperla if you have ideas

maxpumperla commented 2 years ago

@slinlee in the list comprehension above val is of type Array and you try to subset it by typing val[i]. Again, we don't need to support any Python magic /dunder functions (this one would require __getitem__ to work). Instead, we could implement a simple get on the array for each index.

Then again, and this is something we discussed already, Array has a values() method, which I'm reasonably sure returns a Python list (or at least an iterable). So, running e.g. val = val.values() in line 282 should in principle make this work.

maxpumperla commented 2 years ago

what will likely fail, and I have a deja vu there as well, is setting the term_contributions to be a numpy array. Also, shouldn't this rather be max_array += np.array( .... Right now you override max_array in the loop, instead of adding terms.

slinlee commented 2 years ago

what will likely fail, and I have a deja vu there as well, is setting the term_contributions to be a numpy array. Also, shouldn't this rather be max_array += np.array( .... Right now you override max_array in the loop, instead of adding terms.

@maxpumperla this is @brettskymind 's new code from #509 that still hasn't been reviewed yet so if there are logical bugs there, we should fix them too.

maxpumperla commented 2 years ago

@slinlee fixed this for you and added a test replicating the situation.

slinlee commented 2 years ago

run tests