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

Calculation Broken? #138

Closed
TinusNL opened this issue May 26, 2021 · 7 comments
Closed

Calculation Broken? #138

TinusNL opened this issue May 26, 2021 · 7 comments
Assignees

Comments

@TinusNL
Copy link

TinusNL commented May 26, 2021

Hello,
I ran into a problem that I can't seem to be able to fix. The problem is that the calculation function is not running when it needs to. How/Why this happens I am not sure. I was using it yesterday it worked fine. Didn't change anything but somehow it still broke. I tried 2 things that came to my mind. I tested these things by putting console.log("Working!"); in de calculate functions but it doesn't seem to give the reaction it should.

-❌ this.engine.calculate(); - App.vue
-✔️ this.calculate(); - Node.js

I also tried reinstalling the packages but this didn't change anything. I hope that there is a way to fix this issue!

@newcat
Copy link
Owner

newcat commented May 26, 2021

Can you check whether:

  1. your graph has at least one "output" node (a node with only input interfaces and no output interfaces) or that you set the rootNodes manually?
  2. all nodes with calculate functions are connected to the output/rootNodes?

@benkimball
Copy link

I'm having a similar problem. In my case I had forgotten to include the Engine plugin, and once I did that calculation mostly worked. That is, when I manually change an option, the nodes recalculate. What I'm still struggling with is that when the graph loads initially, calculation does not run. I tried calling this.engine.calculate() in my node editor's created() callback, but that appears to cause an infinite loop.

@newcat
Copy link
Owner

newcat commented May 26, 2021

@benkimball What do you mean with "loading initially"? Like calling the load() method?

@benkimball
Copy link

Sure! Let me expand a little on what I'm doing with some code.

Here's my NodeEditor component, which wraps the baklava-editor component:

<template>
  <baklava-editor :plugin="viewPlugin"></baklava-editor>
</template>

<script lang="ts">
import { Editor } from "@baklavajs/core";
import { Engine } from "@baklavajs/plugin-engine";
import { ViewPlugin } from "@baklavajs/plugin-renderer-vue";
import { OptionPlugin } from "@baklavajs/plugin-options-vue";
import { Component, Vue } from "vue-property-decorator";
import { NumberNode, SumNode } from "../nodes";

@Component
export default class NodeEditor extends Vue {
  editor: Editor = new Editor();
  engine: Engine = new Engine(true);
  viewPlugin: ViewPlugin = new ViewPlugin();

  created() {
    this.editor.use(this.engine);
    this.editor.use(this.viewPlugin);
    this.editor.use(new OptionPlugin());
    this.editor.registerNodeType("Number", NumberNode);
    this.editor.registerNodeType("Sum", SumNode);

    const n1 = new NumberNode({ defaultValue: 1 });
    this.editor.addNode(n1);
    n1.position = { x: 100, y: 100 };

    const n2 = new NumberNode({ defaultValue: 1 });
    this.editor.addNode(n2);
    n2.position = { x: 100, y: 300 };

    const sum = new SumNode();
    this.editor.addNode(sum);
    sum.position = { x: 400, y: 200 };

    this.editor.addConnection(
      n1.getInterface("out"),
      sum.getInterface("operand1")
    );
    this.editor.addConnection(
      n2.getInterface("out"),
      sum.getInterface("operand2")
    );
  }
}
</script>

<style lang="scss"></style>

NumberNode is a node that outputs an integer, and SumNode tries to add its two integer inputs and display the sum:

import { Node } from "@baklavajs/core";

export default class NumberNode extends Node {
  type = "NumberNode";
  name = "Number";
  position = { x: 0, y: 0 }; // avoid TypeScript warning

  constructor(
    {
      defaultValue,
      min = undefined,
      max = undefined,
    }: {
      defaultValue: number;
      min?: number;
      max?: number;
    } = { defaultValue: 31 }
  ) {
    super();
    this.addInputInterface("n", "IntegerOption", defaultValue, { min, max });
    this.addOutputInterface("out");
  }

  public calculate() {
    const n = this.getInterface("n").value;
    console.log("calculate", n);
    this.getInterface("out").value = n;
  }
}
import { Node } from "@baklavajs/core";

export default class SumNode extends Node {
  type = "SumNode";
  name = "Sum";
  position = { x: 0, y: 0 }; // avoid TypeScript warning

  constructor() {
    super();
    this.addInputInterface("operand1", "Number");
    this.addInputInterface("operand2", "Number");
    this.addOption("sum", "IntegerOption");
  }

  public calculate() {
    const a = +this.getInterface("operand1").value;
    const b = +this.getInterface("operand2").value;
    this.setOptionValue("sum", a + b);
  }
}

When I start the vue server, and load the app in my browser on localhost, I see the connected nodes, but the SumNode does not display the computed sum from its two inputs until I make a change, for example by clicking to increment or decrement one of the operands.

@newcat
Copy link
Owner

newcat commented May 27, 2021

@benkimball I can reproduce this. Could not find the error yet, but I'll continue trying 😉

@newcat newcat self-assigned this May 27, 2021
@newcat newcat added the bug Something isn't working label May 27, 2021
@newcat
Copy link
Owner

newcat commented Jun 4, 2021

@benkimball Wow it took me way to long to find the bug. It was a race condition in the engine plugin, where the initial calculations due to adding nodes and connections were still running when calling engine.calculate(). Since I designed the engine to be either explicitly called via calculate() or automatically calculating on change, but not both, I didn't check whether a calculation was still running when explicitly calling the calculate() method. This, in conjunction with the async/await inside the calculate function, caused the engine to constantly recalculate because of changes made inside the calculate function.

The bug should be fixed in v.1.8.7

@TinusNL do you still have problems with the calculation?

@newcat newcat removed the bug Something isn't working label Jun 4, 2021
@newcat newcat assigned TinusNL and unassigned newcat Jun 4, 2021
@TinusNL
Copy link
Author

TinusNL commented Jun 4, 2021

I think you fixed it! I am not having anymore problems right now. Thanks for the support.

@TinusNL TinusNL closed this as completed Jun 4, 2021
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

3 participants