generic-github-user / Caesium

General-purpose AI library with NEAT-style genetic algorithm.
https://generic-github-user.github.io/Caesium/src/versions/javascript/projects/network-visualization/
MIT License
1 stars 1 forks source link

Use immutable data structure for network objects #89

Closed generic-github-user closed 5 years ago

generic-github-user commented 5 years ago

Networks should be immutable: each function called on a network should . Data being edited within a network object should not be edited in the object itself, but in a temporary copy, which will be returned as the output of a function. This must be done to prevent the issues caused when cloning a complex JavaScript object with deep properties and methods.

Because of this, network objects should be stored in JavaScript constants so that they are not unintentionally edited.

This will not be needed for the initial release, but it should be done soon.

generic-github-user commented 5 years ago

I had wondered why TensorFlow.js tensor objects were immutable . . . I'm guessing the issue I faced with cloning network objects has something to do with it: #87

generic-github-user commented 5 years ago

This may actually have to be done for the initial release of the project, as the neuroevolution algorithm will not work without a proper cloning system. We could add a temporary system that goes through each layer of the object and deep clones it, or one that clones just the JSON data and adds methods/functions to the object later, but these are both temporary fixes, and it is better to get this out of the way so there is a stable, functioning system in place.

generic-github-user commented 5 years ago

An immutable data structure for objects will almost definitely be needed in the future - if any methods are added to node or connection objects, it will become increasingly impractical to apply temporary cloning solutions.

generic-github-user commented 5 years ago

Can't seem to find a good way to do this in plain JavaScript, I am planning on using Immutable.js. Object.freeze() and Object.assign() are both options, but they only prevent data alteration one level deep in the network objects.

generic-github-user commented 5 years ago

https://benmccormick.org/2016/06/04/what-are-mutable-and-immutable-data-structures-2/

generic-github-user commented 5 years ago

If we only use Object.assign() to make changes to a network object at a single level, we should be fine using plain JavaScript. Including a library into a .js file will be a pain.

generic-github-user commented 5 years ago

Making an object immutable doesn't seem like the best option anymore . . . a deep clone actually seems less painful. Besides, making the object immutable will be just as difficult as cloning it, as we need to clone each individual part of the object to prevent any changes being made to it.

generic-github-user commented 5 years ago

Could create a new network object with a constructor instead, then set the relevant properties and return the object. This seems like it would be inefficient, though.

generic-github-user commented 5 years ago

(Besides, we would need to copy over almost every property anyway - nodes, connections, etc.)

generic-github-user commented 5 years ago

We could use deep cloning to make network objects immutable.

generic-github-user commented 5 years ago

With an immutable data structure, it seems that you're cloning the object (at least to some extent) every time it is modified.

generic-github-user commented 5 years ago

We could only pay attention to one change within the object if we could make sure that the rest of the properties were immutable.

generic-github-user commented 5 years ago

It seems that Immutable.js is geared more towards larger-scale projects, and wouldn't be very efficient for this project: https://stackoverflow.com/questions/39227832/advantages-of-using-immutable-js-over-object-assign-or-spread-operators

generic-github-user commented 5 years ago

Created a new issue for deciding what to do about the data structures: #91

generic-github-user commented 5 years ago

Each property could be frozen using Object.freeze() as it is being created using the network constructor.

generic-github-user commented 5 years ago

Alternatively, each property of the network object could be looped through after the object has been created and frozen.

generic-github-user commented 5 years ago

This could cause undetectable issues if any properties are mistakenly not frozen when network objects are created.

generic-github-user commented 5 years ago

Immutability may reduce speed (and make optimization harder) when performing many operations, for example when evolving and training a network, where speed is a priority. However, Tensorflow.js did not seem to have any significant issues with this. Memory leaks have been a problem, though.

generic-github-user commented 5 years ago

For now, a mutable data structure will be used for network objects: https://github.com/generic-github-user/Caesium/issues/91#issuecomment-415371880

generic-github-user commented 5 years ago

Looks like we will actually be using an immutable data structure . . . references within network objects being cloned are causing issues: #92

generic-github-user commented 5 years ago

A deep freezing function might be an easier solution than freezing each property as the network object is being created.

generic-github-user commented 5 years ago

I have reviewed all of this - there are three main options:

It doesn't seem like using an immutable data structure is actually going to help much for this specific problem. Object freezing could be a solution, but there is still the danger of internal references to different objects remaining when they are not supposed to, or references being broken, especially references to nodes stored in connection sub-objects of the main network objects. The best option seems to be to continue using a mutable data structure, but reference nodes and connections through unique UUIDs instead of JavaScript object references.

generic-github-user commented 5 years ago

Closing this issue - see #93 for future information.