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

feat: add browser for synthesizer #640

Merged
merged 28 commits into from Aug 19, 2021
Merged

feat: add browser for synthesizer #640

merged 28 commits into from Aug 19, 2021

Conversation

hsharriman
Copy link
Contributor

@hsharriman hsharriman commented Aug 5, 2021

Description

First version of a web-app for the synthesizer using React and Material-UI
Screenshot from 2021-08-17 16-02-37

Since this web app has its own dependencies, it has been separated into a new package synthesizer-ui. Notably, It adds a @material-ui/core dependency.

The general purpose of the web-app is to provide a user-interface which problem authors or Penrose users can interact with to generate multiple diagrams, view mutated programs, and export diagrams that they like. This is a first pass, so it covers some core functionalities with opportunities to extend as necessary. The core things that it does:

  • Runs the synthesizer to generate multiple mutated programs based on a file triple
  • Provides a default substance, domain, and style program but they can be altered if the author chooses
  • Provides settings that can be adjusted to configure how many programs, mutations/program, and types of statements can be mutated on
  • Each mutated program displays its CIEE as well as the operations and mutated substance program
  • Clicking on a checkbox stages it for exporting, and clicking on the EXPORT button downloads all staged diagrams as SVGs

Implementation strategy and design decisions

Different sections are divided into various components. Material-UI was chosen because it provides built-in components like sliders--which were necessary for the implementation of some of the settings--which were not to be found in CSS libraries like bootstrap, tailwind, or styled-components.

Examples with steps to reproduce them

  • check out this branch, run yarn to build dependency tree
  • run yarn start and visit localhost:4000

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • New and existing tests pass locally using yarn test
  • I ran yarn docs and there were no errors when generating the HTML site
  • My code follows the style guidelines of this project (e.g.: no ESLint warnings)

Open questions

  • There is a bit of unexpected behavior in the Synthesizer, where if a statement takes a specific type in as args, that type must also be included in the Add type configuration. This is a bit unusual because the user should expect that if they want a specific statement to be included as a possible mutation, for instance Midpoint : Linelike -> Point, they should be able to add only this statement to their configuration without needing to also add Linelike. This behavior has also been observed for Edit mutations.
    • TEMPORARY HACK: To get around this issue temporarily, I edited the public/files/euclidean.txt file so that default Linelike types are rendered with Colors.none (making them invisible). Then, for each of the predicates/constructors for Linelike types, I override the Linelike color to Colors.black. This means that anytime the Synthesizer adds Linelike <x> without further use, it does not render. However, if Linelike <x> is used in any subsequent statements, it will render appropriately.
  • Another bit of unexpected behavior with the Synthesizer: In a ReplaceStmtName edit mutation, the new statement will not recycle the arguments from the old statement. Instead, it creates new arguments. i.e.:
Change EqualLengthMarker1(s1, s2) to EqualLengthMarker2(l0, l1)
(instead of EqualLengthMarker1(s1, s2) --> EqualLengthMarker2(s1, s2)
  • Currently there are copy-pasted versions of examples/euclidean.sty and examples/geometry.dsl in public/files, and we need to adjust the implementation so that the source files can be linked directly without copy/paste. Currently React complains about relative imports from beyond the project root.

@hsharriman hsharriman marked this pull request as draft August 5, 2021 17:23
@codecov
Copy link

codecov bot commented Aug 5, 2021

Codecov Report

Merging #640 (ed60b7e) into main (1ec90f5) will not change coverage.
The diff coverage is 21.42%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #640   +/-   ##
=======================================
  Coverage   66.57%   66.57%           
=======================================
  Files          44       44           
  Lines        7103     7103           
  Branches     1337     1336    -1     
=======================================
  Hits         4729     4729           
  Misses       2365     2365           
  Partials        9        9           
Impacted Files Coverage Δ
packages/core/src/compiler/Domain.ts 86.84% <ø> (ø)
packages/core/src/compiler/Style.ts 87.42% <0.00%> (+0.06%) ⬆️
packages/core/src/synthesis/Mutation.ts 22.97% <0.00%> (ø)
packages/core/src/synthesis/Synthesizer.ts 20.70% <10.00%> (-0.06%) ⬇️
packages/core/src/compiler/Substance.ts 90.35% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ec90f5...ed60b7e. Read the comment docs.

@hsharriman hsharriman changed the title feat: adding browser for synthesizer feat: add browser for synthesizer Aug 12, 2021
@hsharriman hsharriman requested a review from wodeni August 13, 2021 23:57
Copy link
Member

@wodeni wodeni left a comment

Choose a reason for hiding this comment

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

Great job! I just went through the code and dropped some comments below. I'll play around with the UI a bit later :D.

~Seems like the CI is failing for some reason. Can you try running yarn build locally and see what errors you are getting? I'm guessing something is off with react-scripts?~

Fixed now!

packages/synthesizer-ui/README.md Outdated Show resolved Hide resolved
packages/synthesizer-ui/estrella.js Outdated Show resolved Hide resolved
packages/synthesizer-ui/package.json Outdated Show resolved Hide resolved
packages/synthesizer-ui/src/App.test.tsx Outdated Show resolved Hide resolved
packages/synthesizer-ui/src/components/Grid.tsx Outdated Show resolved Hide resolved
packages/synthesizer-ui/src/components/Gridbox.tsx Outdated Show resolved Hide resolved
packages/synthesizer-ui/src/components/Gridbox.tsx Outdated Show resolved Hide resolved
packages/synthesizer-ui/src/components/Settings.tsx Outdated Show resolved Hide resolved
mutationCount: newValue as [number, number],
},
});
console.log(this.state.setting?.mutationCount);
Copy link
Member

Choose a reason for hiding this comment

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

Consider using consola and controlling the log level?

Copy link
Member

@wodeni wodeni left a comment

Choose a reason for hiding this comment

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

💯

@wodeni wodeni marked this pull request as ready for review August 19, 2021 12:28
@wodeni wodeni merged commit 2d81a55 into main Aug 19, 2021
@wodeni wodeni deleted the synthesizer-browser branch August 19, 2021 12:28
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