Skip to content

Conversation

@hluecking1
Copy link

No description provided.

@jpoeppel
Copy link
Owner

jpoeppel commented Apr 9, 2018

Hi Hendrik, sorry for only getting around this now. I have only 2 questions regarding this PR:
Is there a reason for you deleting some of the tests? From looking over the code, I could not find the reason for removing those tests.

Also, you changed the Dynamic Bayesian Network quite a lot, instead of creating a new Dynamic Decision Network. Does the old DBN still work?

Depending on the answer, I might just cherry pick parts of this PR and merge it, instead of the entire PR so that it is easier for me to merge the Decision Networks back into to root repository.

@hluecking1
Copy link
Author

Hey Jan,
I've added the old network again (sorry i dont know why it was gone). I did not remove any tests.

Copy link
Owner

@jpoeppel jpoeppel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Hendrik,
thanks for the quick reply! When I check, the changed files, it shows some test functions to be removed. Maybe you would need to rebase first. But if that was not intentional, I can just take care of that when merging, so you would not need to change that, just wanted to make sure that there was no special reason behind that.

class ContinousNode(unittest.TestCase):
pass

class UtilityNode(unittest.TestCase):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github claims these lines here have been removed.

self.assertTrue(v in f.values[p])


def test_create_from_utility_node(self):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are also some that appear to be missing after the merge

@hluecking1
Copy link
Author

Yeah it was not intentional. You can take care of it !

@jpoeppel
Copy link
Owner

jpoeppel commented Apr 9, 2018

Great thanks for the quick reply! I will try to find the time to merge it (also into the base repository) within the next couple of weeks.

Hendrik Luecking added 8 commits April 11, 2018 10:49
…: testing max sum and adding transition probability
…ples. For every example to get the optimal decision both algorithms are now used (classical and max_sum)
…gaussian filter and the current believe state
… in calculating the inner product, when a decision Rule is given for ALL Decision Nodes. Transition Matrix can now also be a dictionary (better readability). Added the possibility to set the decision Nodes to the best action based on a skill believe of the parent node: This feature still has to be updated
…hm (For the next skill). Bugfixing: Deleted inter edges in the unrolled net after doing update. The Child of the inter edge Relationship had an exponentially growing CPD which lead to Memory Problems. Obviously we dont want the next node to have the inter_edges internalized in the unrolled net otherwise we cant make proper updates with the same Dimension we had in the beginning. The Unrolled function looks a bit ugly now i try to make it more compact in the future
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