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

Incorrect update of errors #27

Closed
hasanbalci opened this issue Jul 18, 2024 · 5 comments
Closed

Incorrect update of errors #27

hasanbalci opened this issue Jul 18, 2024 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@hasanbalci
Copy link
Contributor

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 hasanbalci added the bug Something isn't working label Jul 18, 2024
@hasanbalci
Copy link
Contributor Author

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

YusufZiyaOzgul added a commit that referenced this issue Aug 7, 2024
@YusufZiyaOzgul
Copy link
Collaborator

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
Copy link
Contributor Author

hasanbalci commented Aug 7, 2024

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
Copy link
Contributor

ugurdogrusoz commented Aug 9, 2024

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
Copy link
Contributor Author

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

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

3 participants