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: inline comparison operators #1257

Merged
merged 15 commits into from Jan 25, 2023
Merged

feat: inline comparison operators #1257

merged 15 commits into from Jan 25, 2023

Conversation

liangyiliang
Copy link
Contributor

@liangyiliang liangyiliang commented Jan 23, 2023

Description

Resolves #1168 .

We added the support for inline comparison operators. In particular,

  • a > b is the syntax sugar for greaterThan(a, b)
  • a == b is the syntax sugar for equal(a, b)
  • a < b is the syntax sugar for lessThan(a, b)

This applies to both constraints and objectives.

Implementation strategy and design decisions

We modified the parser so that constraints and objective declarations now look like

<constr> ::= ensure <body> <staged layout>+
   <obj> ::= encourage <body> <staged layout>+

  <body> ::= <identifier> ( <expr_list> )  // Function Call
           | <expr> <op> <expr>            // Inline Comparison

    <op> ::= ==     // Equal
           | <      // Less Than
           | >      // Greater Than

In other words, the of a constraint or objective now can either be a Function Call or an Inline Comparison.

In the Style compiler where we handle constraints and objectives, we proceed as before when we encounter a Function Call. When we encounter an Inline Comparison, on the other hand, we handle it as if it is a Function Call, with the corresponding function name (lessThan, greaterThan, or equal) and the two operands as parameters to the function.

Tests have been added to the parser and compiler.

The docs site has been updated to include this syntactic sugar. Furthermore, we modified the tree-venn and tree-tree example to utilize this syntactic sugar.

Examples with steps to reproduce them

See examples tree-venn and tree-tree with applications of this syntactic sugar.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new ESLint warnings
  • I have reviewed any generated registry diagram changes

@liangyiliang liangyiliang changed the title Added failing test cases feat: inline comparison operators Jan 23, 2023
@github-actions
Copy link

github-actions bot commented Jan 23, 2023

± Registry diff

📊 Performance

Key

Note that each bar component rounds up to the nearest 100ms, so each full bar is an overestimate by up to 400ms.

     0s   1s   2s   3s   4s   5s   6s   7s   8s   9s
     |    |    |    |    |    |    |    |    |    |
name ▝▀▀▀▀▀▀▀▀▀▀▀▚▄▄▄▄▄▄▄▄▄▞▀▀▀▀▀▀▀▀▀▀▀▀▚▄▄▄▄▄▄▄▄▄▖
      compilation labelling optimization rendering

Data

                                        0s   1s   2s   3s   4s   5s   6s   7s   8s
                                        |    |    |    |    |    |    |    |    |
3d-projection-fake-3d-linear-algebra    ▝▞▖
allShapes-allShapes                     ▝▚▚▄▖
arrowheads-arrowheads                   ▝▚▚
caffeine-structural-formula             ▝▀▀▀▚▀▖
center-shrink-circle-animation          ▝▞▖
circle-example-euclidean                ▝▀▚▀▀▀▖
collinear-euclidean                     ▝▀▞▖
congruent-triangles-euclidean           ▝▀▀▚▚
continuousmap-continuousmap             ▝▚▚
cubic-bezier-cubic-bezier               ▝▚▀▀▚
glutamine-molecules-basic               ▝▀▚▚
half-adder-distinctive-shape            ▝▀▞▚
hypergraph-hypergraph                   ▝▀▀▀▞▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▚
incenter-triangle-euclidean             ▝▀▀▞▖
lagrange-bases-lagrange-bases           ▝▚▚
midsegment-triangles-euclidean          ▝▀▞▖
mobius-mobius                           ▝▞▖
non-convex-non-convex                   ▝▀▞▖
one-water-molecule-atoms-and-bonds      ▝▞▖
parallel-lines-euclidean                ▝▀▞▖
persistent-homology-persistent-homology ▝▀▀▚▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▄
points-around-line-shape-distance       ▝▀▚▚
points-around-polyline-shape-distance   ▝▀▀▞▖
points-around-star-shape-distance       ▝▀▀▀▚▚
siggraph-teaser-euclidean-teaser        ▝▀▚▚
small-graph-disjoint-rect-line-horiz    ▝▀▀▞▖
small-graph-disjoint-rects              ▝▚▚
small-graph-disjoint-rects-large-canvas ▝▞▖
small-graph-disjoint-rects-small-canvas ▝▚▚
tree-tree                               ▝▀▄▚
tree-venn                               ▝▀▚▚
tree-venn-3d                            ▝▀▞▄▖
two-vectors-perp-vectors-dashed         ▝▚▚
vector-wedge-exterior-algebra           ▝▞▖
wet-floor-atoms-and-bonds               ▝▀▚▀▀▖
word-cloud-example-word-cloud           ▝▀▚▚
wos-laplace-estimator-walk-on-spheres   ▝▀▀▞▖
wos-nested-estimator-walk-on-spheres    ▝▀▀▚▀▀▀▀▀▀▀▀▖
wos-offcenter-estimator-walk-on-spheres ▝▀▚▀▀▀▀▖
wos-poisson-estimator-walk-on-spheres   ▝▀▚▀▀▖

@cloudflare-pages
Copy link

cloudflare-pages bot commented Jan 23, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 49781b2
Status: ✅  Deploy successful!
Preview URL: https://761ec6e1.penrose-72l.pages.dev
Branch Preview URL: https://inline-constraint.penrose-72l.pages.dev

View logs

@codecov
Copy link

codecov bot commented Jan 24, 2023

Codecov Report

Merging #1257 (49781b2) into main (e1944ab) will decrease coverage by 0.02%.
The diff coverage is 65.00%.

@@            Coverage Diff             @@
##             main    #1257      +/-   ##
==========================================
- Coverage   63.44%   63.43%   -0.02%     
==========================================
  Files          61       61              
  Lines        7176     7212      +36     
  Branches     1667     1680      +13     
==========================================
+ Hits         4553     4575      +22     
- Misses       2540     2553      +13     
- Partials       83       84       +1     
Impacted Files Coverage Δ
packages/core/src/utils/Util.ts 48.57% <0.00%> (-1.28%) ⬇️
packages/core/src/compiler/Style.ts 68.51% <100.00%> (+0.51%) ⬆️
packages/core/src/index.ts 43.54% <0.00%> (-1.83%) ⬇️
packages/core/src/contrib/Constraints.ts 82.88% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@liangyiliang liangyiliang marked this pull request as ready for review January 24, 2023 22:02
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.

Looks good! This will actually improve the editing experience for Style dramatically for me (and I'm sure many others!). Just had a minor comment about parser tests below. Also I'll try and push some refactoring on the geometry domain

packages/core/src/parser/StyleParser.test.ts Show resolved Hide resolved
@wodeni
Copy link
Member

wodeni commented Jan 25, 2023

Refactored all lessThan and equal in euclidean.sty to use the new inline operators! Local tests seemed to work very well. Some variations might need tweaks for gallery, which I'll do after CI reports the diffs.

@wodeni
Copy link
Member

wodeni commented Jan 25, 2023

BTW it'd be good to open issues about (1) chaining and (2) other logical operators (eg ||, >= etc). This PR technically doesn't resolve all the things in #1168, but I somewhat prefer closing issues sooner and open more specific followups for the health of the repo.

Co-authored-by: Wode "Nimo" Ni <wn2155@columbia.edu>
@samestep
Copy link
Collaborator

Looks super cool, thanks! Looking at the code now... quick question though, why does the tree-venn example change?

@liangyiliang
Copy link
Contributor Author

liangyiliang commented Jan 25, 2023

why does the tree-venn example change?

We changed smallerThan(x.icon, y.icon) with x.icon.r < y.icon.r, equivalent to lessThan(x.icon.r, y.icon.r).

smallerThan has a default nonzero padding of 0.4. lessThan's default padding is 0.

packages/core/src/__tests__/overall.test.ts Outdated Show resolved Hide resolved
packages/core/src/compiler/Style.ts Outdated Show resolved Hide resolved
packages/core/src/compiler/Style.ts Show resolved Hide resolved
@samestep
Copy link
Collaborator

We changed smallerThan(x.icon, y.icon) with x.icon.r < y.icon.r, equivalent to lessThan(x.icon.r, y.icon.r).

smallerThan has a default nonzero padding of 0.4. lessThan's default padding is 0.

Gotcha; but why does that change the colors?

@samestep
Copy link
Collaborator

I guess the reason I'm asking is that I feel like the colors in the variation as of this PR don't look great; if that example does have to change, could you modify the variation to get nicer colors?

@liangyiliang
Copy link
Contributor Author

Gotcha; but why does that change the colors?

I think the overall energy function changed, which means a different optimization ...

But in reality now that I think about it, I shouldn't really touch smallerThan - since I think the idea of having a padding makes sense ... I will revert the change to tree-venn

Copy link
Collaborator

@samestep samestep left a comment

Choose a reason for hiding this comment

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

Great, thanks Leo!

Copy link
Member

@joshsunshine joshsunshine left a comment

Choose a reason for hiding this comment

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

Looks great!

@liangyiliang liangyiliang merged commit b3c7c2f into main Jan 25, 2023
@liangyiliang liangyiliang deleted the inline-constraint branch February 24, 2023 21:32
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.

Inline comparison operators for constraints and objectives
4 participants