Incorrect update of errors

iVis-at-Bilkent / sybvals

SyBValS: Systems Biology Validation Service

MIT License

2 stars 0 forks source link

Incorrect update of errors #27

Closed hasanbalci closed 3 months ago

hasanbalci commented 4 months ago

If we validate this file: new_file_7.nwt.txt, it shows pd10124 error which is correct. When we try to fix it, it is fixed but still exist in the error list as unresolved. Also, this fix introduces another error but it is not listed in the error list.

hasanbalci commented 3 months ago

Current status of this issue is as follows (please see the video):

While resolving errors second time, for Error3, even though a macromolecule is selected, the new edge is added from process to process itself.
In addition, for the same error, even though it is resolved by selecting one of the macromolecules from the given choices, fix explanation indicates "Fix of another error resolved this error.".

https://github.com/user-attachments/assets/3417926c-996b-43d1-b86d-fb1d5ecde218

YusufZiyaOzgul commented 3 months ago

Current status of this issue is as follows (please see the video):

* While resolving errors second time, for Error3, even though a macromolecule is selected, the new edge is added from process to process itself.

* In addition, for the same error, even though it is resolved by selecting one of the macromolecules from the given choices, fix explanation indicates "Fix of another error resolved this error.".

Screen.Recording.2024-08-05.at.13.48.02.mov

@hasanbalci @ugurdogrusoz bugs are fixed. First bug is related to not updating default choice correctly for pd10141 error type and now default choice is updated correcty. Second issue occurs since fix explanations for recently resolved error types not added yet, I added explanations for pd10141, pd10127, pd10128, pd10110. Now, it does not occur.

hasanbalci commented 3 months ago

The main two problems I mentioned seem resolved. But while testing, some new ideas came to my mind (I don't know if they are easy to do):

While solving pd10141 (All process nodes should have at least one input and at least one ouput), maybe we can check whether there is a orphaned EPN or not. If there is, we can suggest that EPN as the default choice. In this way, the error pd10131 (EPNs should not be orphaned (i.e. they must be associated with at least one arc)) caused by the orphaned EPN will also be solved automatically.
Again for the error pd10141, if a process node has an input as EPN but doesn't have an output, we suggest nearby EPNs as alternatives, but the input EPN is selected as the default option (probably because it is the nearest?). But it is unlikely that the input and output of a process will be the same EPN (even though it is possible in terms of validation rules). Maybe we can keep that EPN in the alternatives, but not select as the default option if there is any other option.

@ugurdogrusoz What do you think?

ugurdogrusoz commented 3 months ago

I suggest we leave this improvement for a next round. Let's get a first version released and paper submitted first. Let's collect all potential future improvements in a separate issue?

hasanbalci commented 3 months ago

I moved the last suggestions to a new issue #31 as future improvements, so I'm closing this.