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

Review of NMF #5

Open
chrissi007 opened this issue Jul 4, 2019 · 1 comment
Open

Review of NMF #5

chrissi007 opened this issue Jul 4, 2019 · 1 comment

Comments

@chrissi007
Copy link
Contributor

NMF

NMF demonstrates his implicit incrementalization abilities to automatically create a binary decision tree from a truth table.

  • Only looks at the BDT structure and creates this ones
  • No readme with prerequisites for installation (Installation of DotNet.Core.App 2.1 or higher is necessary)
  • Works after installing Dot.Net.Core.App 2.2
  • Prints load, initial, initialization time and memory values
  • Config.json only tests the models: "Test.ttmodel", "GeneratedI4O2Seed42.ttmodel", "GeneratedI8O2Seed68.ttmodel", "GeneratedI8O4Seed68.ttmodel"
  • Does not save the result models
  • Synchronization of Ports, BDD, and TruthTable classes is easy to understand
  • Mechanism for leaf nodes and subtrees should be explained better, hard to understand, I would prefer a flow diagram
  • Explain more the helper classes for synchronization as TreeAssignment, because they are the important elements in this transformation
  • Tree is incrementally created from each row, but cannot be proven if the results are correct because they are not saved
  • Has the tree dead ends if the TruthTable is not complete?
  • Has the test.ttmodel dead ends after the algorithm?
  • Good approach very interesting!

Structure

  • Abstract and Intro good
  • Background a bit general the helper classes are missing on which the complete transformations depends on
  • Solution I think a bit to long
  • Do you want to add an evaluation?
  • Highlight the advantages and disadvantages of the solution more in the reflection or conclusion section.

Small remarks

  • Page 1 last line: papes -> paper

Metrics

  1. Does the solution use the provided benchmark structure, i.e., is a batch execution supported?
    -> Yes but installation of DotNet.Core.App 2.1 or higher is needed
  2. Can all models be transformed within 10 minutes?
    -> Not the largest model (one on our machine)
  3. What kind of result is produced? BDT, BDD, with fix order
    -> Looks like BDT with fix order
  4. Is the transformation correct?
    -> NMF does not store the bdd models
    -> Cannot prove if the results are correct
  5. Metrics
    -> no metrics are computed
    -> From the descriptions it sound like a complete BDT without optimization
@georghinkel
Copy link
Contributor

As a response, I was not aware that the case description had changed. Therefore, the NMF solution only knows the original BDD model. Likewise, the solution does not print any more metrics although I agree that this would be a useful extension (and also easy to implement).

Regarding dead ends, yes, the solution wil produce dead ends in the BDD if the original truth table is not complete and sometimes even if it is. This is because there is no post-processing done. The solution does not really put a focus on optimality of the result, it is rather an excercise of integrating imperative code blocks into an incrementalized transformation.

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

No branches or pull requests

2 participants