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

Redesign BooleanNetwork/RegulatoryGraph API with better integrity checks #21

Open
daemontus opened this issue Apr 7, 2021 · 1 comment
Labels
bug Something isn't working
Milestone

Comments

@daemontus
Copy link
Member

Public methods for manipulation of Boolean networks were originally designed just for the .aeon format parser. As such, they are missing some safety checks (like verifying that an update function actually uses parameters which are present in the network).

At least, we should fix those safety issues (every change to the network should probably trigger some kind of full consistency check). Additionally, it would be good to design the API so that a BN can be modified more easily. For example, getting a mutable reference of the inner regulatory graph would help a lot (since then we can add regulations even after a BN is constructed from the graph).

@daemontus daemontus added the bug Something isn't working label Apr 7, 2021
@daemontus daemontus added this to the 1.0.0 milestone Dec 22, 2021
@daemontus
Copy link
Member Author

This is a much bigger issue than it may initially seem. The problem is largely a bad design in the original API, where a lot of operations that can fail have no way to return an error. So the only option at the moment is to panic. Also, a lot of checks which are performed by the parser should in fact be performed by BooleanNetwork and RegulatoryGraph instead, otherwise one can easily create a malformed network using our current API.

An (incomplete) list of problems that need to be addressed:

  • RegulatoryGraph::new should check if variables are unique and that their names are valid.
  • If the regulation is not used, RegulatoryGraph::remove_regulation should be available. Maybe a "force" variant can be also added, which removes the variable from other update functions if used.
  • RegulatoryGraph::add_variable and RegulatoryGraph::remove_variable should be also available, again with all the necessary safety checks. Variable ID can be reused once deleted.
  • The regulation properties (observability, monotonicity) should be editable through a mutable reference.
  • BooleanNetwork should be also able to remove and add parameters, and update their arity.
  • BooleanNetwork::set_update_function must verify that the update function is valid within the network at the time of insertion.
  • It is probably time to migrate away from String as error type to some custom enum with a sensible to_string() implementation. Other tools may want to display our errors differently.

@daemontus daemontus changed the title BooleanNetwork.add_update_function should check parameter consistency Redesign BooleanNetwork/RegulatoryGraph API with better integrity checks Dec 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant