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

Fix/inconsistent location invariant #94

Merged
merged 19 commits into from
Mar 9, 2023

Conversation

Brandhoej
Copy link
Contributor

@Brandhoej Brandhoej commented Nov 29, 2022

Fixes #62 where the inconsistent location has an incorrect invariant. The fix for this was to introduce a global clock which, upon creation of a Quotient instance, is searched for. If this clock exists then it is reused in a new inconsistent location otherwise one is created. Also, flaky tests were happening as some tests at random times tried to reduce a terminal CDD this has been fixed by adding several checks in CDD for shortcut computations.

Description

  • Quotient: Now reuses clock used by inconsistent locations.
  • UniqueNamedContainer: Has been updated to allow for global named items in its collection.
  • Automaton: Making an automaton input enabled has been refactored and terminal CDD, which could be encountered, are now handled gracefully. With the additions in CDD I don't think the isTerminal check is required, but I still prefer having it.
  • QuotientTest: Now has tests to check whether clocks are being reused.

Opens

  • I think the introduction of global clocks introduce the requirement for explicitly designing our scope rules.
  • Instead of using names for an Automaton I think we should use a symbol from a symboltable.

Related Issues

Closes #62

@Brandhoej Brandhoej self-assigned this Nov 29, 2022
@Brandhoej Brandhoej marked this pull request as ready for review January 16, 2023 20:44
@Brandhoej Brandhoej requested a review from magoorden January 16, 2023 20:44
Copy link
Contributor

@magoorden magoorden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have done a round of review.

For next time: reviews are (in general) easier when they are smaller. Now you solve multiple issues in the same merge request.

src/logic/Refinement.java Show resolved Hide resolved
src/models/Automaton.java Outdated Show resolved Hide resolved
src/models/Automaton.java Outdated Show resolved Hide resolved
src/models/Automaton.java Outdated Show resolved Hide resolved
src/models/CDD.java Show resolved Hide resolved
test/features/UniversityTest.java Show resolved Hide resolved
test/logic/LocationTest.java Show resolved Hide resolved
test/logic/LocationTest.java Show resolved Hide resolved
test/logic/QuotientTest.java Outdated Show resolved Hide resolved
test/logic/QuotientTest.java Outdated Show resolved Hide resolved
src/models/CDD.java Outdated Show resolved Hide resolved
src/models/CDD.java Show resolved Hide resolved
src/models/CDD.java Outdated Show resolved Hide resolved
src/models/UniqueNamedContainer.java Outdated Show resolved Hide resolved
@ulriknyman
Copy link
Contributor

ulriknyman commented Feb 6, 2023 via email

@Brandhoej Brandhoej mentioned this pull request Feb 12, 2023
Copy link
Contributor

@magoorden magoorden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the improvement. Just some minor stuff I noticed.

src/models/CDD.java Outdated Show resolved Hide resolved
src/models/CDD.java Outdated Show resolved Hide resolved
src/models/CDD.java Outdated Show resolved Hide resolved
src/models/CDD.java Show resolved Hide resolved
src/models/UniqueNamedContainer.java Outdated Show resolved Hide resolved
@Brandhoej Brandhoej merged commit 14b029b into Ecdar:main Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The inconsistant location produced by the quotient has incorrect invariant
3 participants