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

Pretty print model as Essence #582

Merged
merged 4 commits into from
Jan 10, 2025
Merged

Conversation

niklasdewally
Copy link
Collaborator

@niklasdewally niklasdewally commented Jan 10, 2025

Using the pretty printers implement Display for Model to print it in Essence syntax.

Continuation of #568 , in which I added a pretty printer for variable declarations.

Changelog

impl Display for Model, printing it as Essence

Implement Display for Model to print it in Essence.

Using this, change CLI logging to print models in essence instead of
JSON.


tester: pretty print initial and rewritten models in human rule trace

  • During integration testing, pretty print the initial and rewritten
    models in Essence in the human rule trace.

  • Change verbose print statements in the integration tester to pretty
    print the model as Essence.

Currently we only have the rewritten model output in Debug format;
having this in Essence should help with debugging.


refactor!: Change type of SymbolTable from HashMap to BTreeMap

Change type of SymbolTable from HashMap to BTreeMap.

BTreeMap gives variables an ordering in the symbol table, allowing
them to be printed in a deterministic order. It also implements Eq,
and Hash, which will allow us to compare symbol tables to each other
in the future.

Previously, with a HashMap, the key ordering was unspecified, so they
could be printed in any order. The keys printing in a different order in
the expected and generated test files broke some tests.

We plan to convert more HashMaps to BTreeMaps in the future for
similar reasons. For example, using it for solution sets will allow us
to compare them easier (due to implementing Eq and Hash) (#531)

Implement `Display` for `Model` to print it in Essence.

Using this, change CLI logging to print models in essence instead of
JSON.
* During integration testing, pretty print the initial and rewritten
  models in Essence in the human rule trace.

* Change verbose print statements in the integration tester to pretty
  print the model as Essence.

Currently we only have the rewritten model output in `Debug` format;
having this in Essence should help with debugging.
@niklasdewally niklasdewally self-assigned this Jan 10, 2025
@niklasdewally niklasdewally changed the title impl Display for Model, printing it as Essence Pretty print model as Essence Jan 10, 2025
Change type of `SymbolTable` from `HashMap` to `BTreeMap`.

`BTreeMap` gives variables an ordering in the symbol table, allowing
them to be printed in a deterministic order. It also implements `Eq`,
and `Hash`, which will allow us to compare symbol tables to each other
in the future.

Previously, with a `HashMap`, the key ordering was unspecified, so they
could be printed in any order. The keys printing in a different order in
the expected and generated test files broke some tests.

We plan to convert more `HashMaps` to `BTreeMaps` in the future for
similar reasons. For example, using it for solution sets will allow us
to compare them easier (due to implementing `Eq` and `Hash`) [#531].
@niklasdewally niklasdewally marked this pull request as ready for review January 10, 2025 16:04
@niklasdewally
Copy link
Collaborator Author

niklasdewally commented Jan 10, 2025

FYI: @Soph1514 @ewilbert7

@niklasdewally
Copy link
Collaborator Author

I'll fix the new clippy warnings before merging

@ozgurakgun
Copy link
Contributor

lgtm, except the hygiene issue

@niklasdewally niklasdewally force-pushed the nik/pr/tester-print-final-model branch from d086e42 to fe3e1ee Compare January 10, 2025 16:17
@niklasdewally niklasdewally merged commit a446b2e into main Jan 10, 2025
14 checks passed
@niklasdewally niklasdewally deleted the nik/pr/tester-print-final-model branch January 10, 2025 16:21
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.

None yet

2 participants