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

What is the diagram option "Reaction level" supposed to do? #2017

Closed
cmnrd opened this issue Sep 22, 2023 · 4 comments · Fixed by #2019
Closed

What is the diagram option "Reaction level" supposed to do? #2017

cmnrd opened this issue Sep 22, 2023 · 4 comments · Fixed by #2019
Assignees
Labels
diagrams Problems with diagram synthesis question Further information is requested

Comments

@cmnrd
Copy link
Collaborator

cmnrd commented Sep 22, 2023

I stumbled across the diagram option "Reaction level" today. However, it does not seem to do anything. I tested this on various programs, but the diagrams did not change when checking or unchecking this option.

@cmnrd cmnrd added question Further information is requested diagrams Problems with diagram synthesis labels Sep 22, 2023
@a-sr
Copy link
Collaborator

a-sr commented Sep 22, 2023

@edwardalee added this option, he can probably explain best what it does.
As far as I know, it shows levels that indicate orderings due to dependencies and exposes potential for parallelization of reactions.
What I don't know is how it works. I guess the option will only display something if there was some kind of compilation producing the necessary information.

@edwardalee
Copy link
Collaborator

I suspect this no longer works because of this change by @petervdonovan in ReactionInstance.java:

    // Force calculation of levels if it has not been done.
FIXME
    // FIXME: Comment out this as I think it is redundant.
    //  If it is NOT redundant then deadline propagation is not correct
    // parent.assignLevels();

Testing this hypothesis, however, seems rather time consuming now that everything is in separate repos. I don't have time to check this.

@edwardalee
Copy link
Collaborator

That said, maybe we don't really need this feature? Perhaps easiest to just remove it.

@cmnrd
Copy link
Collaborator Author

cmnrd commented Sep 22, 2023

I was also wondering if it is worth repairing. I'll open a PR removing this synthesis option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
diagrams Problems with diagram synthesis question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants