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.

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

bug fix and improvement #7

Merged
merged 5 commits into from
Apr 26, 2023
Merged

bug fix and improvement #7

merged 5 commits into from
Apr 26, 2023

Conversation

saeedark
Copy link
Contributor

  1. adds counter roll for the buffer_roll method
  2. fixes no execution list when the network object itself has behavior
  3. more efficient event recorder and support for multidimensional data
  4. fix the remove_behavior bug for not passing the parent object.

Execution of super() for Network objects with non empty behavior
argument raises:

'Network' object has no attribute 'sorted_behavior_execution_list'

since 'sorted_behavior_execution_list' hasn't been created beforehand
to add network's behaviors into.
adds N dimensional data support.
more efficient.
`_remove_behavior_from_sorted_execution_list` can now accept a behavior
without its parent object used in `remove_behavior` method in
NetworkObjectBase object.
@gitmv
Copy link
Collaborator

gitmv commented Apr 25, 2023

Hello, thanks for you contribution.
The first three updates seem to work and i will merge them, but the fourth one crashes because the arguments are switched.

Can i ask why you want to change the _remove_behavior_from_sorted_execution_list function in the first place, so that it works with no parent object?

The function is only used internally by the remove_behavior function in the NetworkObjectBase class. if you execute it without the parent, it can also remove multiple behaviours(if they are the same instance) of different objects (unlikely but it is possible that someone attaches the same behaviour instance twice) from the sorted_behavior_execution_list. The problem here is, that this behaviour is not only stored in the execution list, but also in the object.behaviours list. If you call net._remove_behavior_from_sorted_execution_list(beh), it is removed from the execution list, and not called anymore during a simulation run, but it would still be tag searchable and it would still appear in the obj.behaviours list.

Therefore _remove_behavior_from_sorted_execution_list should not be called directly from outside and behaviours should always be removed via the obj.remove_behavior(beh) function so that it is removed completely.

Do you have some reason why you want to use it this way?

@saeedark
Copy link
Contributor Author

saeedark commented Apr 25, 2023

Hello,
Sorry, I didn't think that through. my bad.

But _remove_behavior_from_sorted_execution_list always removes the last instance of a behavior in an object. In that unlikely situation(same instance, multiple keys) removing a behavior with its key (via remove_behavior ) may lead to a wrong order of execution.

@saeedark
Copy link
Contributor Author

the last commit should make _remove_behavior_from_sorted_execution_list specific to remove exact behavior.

@gitmv gitmv merged commit a99bbc1 into trieschlab:master Apr 26, 2023
@gitmv
Copy link
Collaborator

gitmv commented Apr 26, 2023

Ok this looks good!
The merge seems to work.

I appreciate it and feel free to improve the project further if you have any ideas or if you find bugs of some kind. I also think that the version with the torch core is a great idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants