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

Subdiagram Improvements #113

Open
srush opened this issue Oct 4, 2022 · 11 comments
Open

Subdiagram Improvements #113

srush opened this issue Oct 4, 2022 · 11 comments
Labels
enhancement New feature or request

Comments

@srush
Copy link
Collaborator

srush commented Oct 4, 2022

Subdiagrams come up a lot more than I thought. But it is a little awkward to work with these at the low level. There are a couple of nice functions (locations, with_name, qualification) that we could add.

https://diagrams.github.io/doc/manual.html#named-subdiagrams

Relatedly, I would like to be able to have text positioned above an arrow. Not sure the best way to do this, but maybe there is some way to place named control points.

@danoneata danoneata added the enhancement New feature or request label Oct 6, 2022
@danoneata
Copy link
Collaborator

Okay, looking at the first example in their tutorial

dia1 = (square 1 # translateY 4 # named "bob" ||| circle 1 # named "joe")
     # connect "bob" "joe"
dia2 = (square 1 # named "bob" # translateY 4 ||| circle 1 # named "joe")
     # connect "bob" "joe"

example :: Diagram B
example = hsep 2 [dia1, dia2]

I see that our implementation has a different behaviour:

from chalk import *

dia1 = square(1).translate(0, -4).named("bob") | circle(1).named("joe")
dia1 = dia1.connect("bob", "joe")

dia2 = square(1).named("bob").translate(0, -4) | circle(1).named("joe")
dia2 = dia2.connect("bob", "joe")

dia = hcat([dia1, dia2], sep=2)

o

I'll need to check what's going on.

@srush
Copy link
Collaborator Author

srush commented Oct 7, 2022

This seems very subtle. At first read I had it backwards in my head what the behavior would be. It's quite counter intuitive to me that

square 1 # translateY 4 # named "bob"

Would connect to the old value? Is it because of the way their semantics work? It seems from their hierarchy names propagate like style.

https://hackage.haskell.org/package/diagrams-core-1.5.0.1/docs/Diagrams-Core-Types.html#g:4

@srush
Copy link
Collaborator Author

srush commented Oct 7, 2022

Oh I see the problem. We are using envelope.center in connect which is an approximation of the midpoint of the diagram content along the x/y axes. They are using the true local origin (0, 0) of the subdiagram. We actually don't have access to that in our code.

Proposal:

  • Remove get_subdiagram_trace and get_subdiagram_envelope
  • Add get_subdiagram
  • Make a Subdiagram data class that privately stores a diagram, a transformation, and a style (DownAnnots) that will be applied to that subdiagram
  • Expose a location method that gives the local origin (computed from the transformation)
  • Expose a get_trace method give the located trace (computed from the transformation) and a boundary_from helper
    https://hackage.haskell.org/package/diagrams-lib-1.4.5.3/docs/src/Diagrams.Trace.html#boundaryFrom

(Not sure what to do about envelope. Maybe you just get access to the transformed version of that. Might be useful to know the radius of the subfigure as presented in the final diagram? But should it be located from the subdiagram origin or the current diagram origin.)

Edit: Sometimes Haskell is so clear. The envelope of the a subdiagram is what we were returning. We should just do the same thing in Subdiagram.

instance (OrderedField n, Metric v, Monoid' m)
      => Enveloped (Subdiagram b v n m) where
  getEnvelope (Subdiagram d a) = transform (transfFromAnnot a) $ getEnvelope d

instance (OrderedField n, Metric v, Semigroup m)
      => Traced (Subdiagram b v n m) where
  getTrace (Subdiagram d a) = transform (transfFromAnnot a) $ getTrace d

instance (Metric v, OrderedField n)
      => HasOrigin (Subdiagram b v n m) where
  moveOriginTo = translate . (origin .-.)

instance Transformable (Subdiagram b v n m) where
  transform t (Subdiagram d a) = Subdiagram d (transfToAnnot t <> a)

-- | Get the location of a subdiagram; that is, the location of its
--   local origin /with respect to/ the vector space of its parent
--   diagram.  In other words, the point where its local origin
--   \"ended up\".
location :: (Additive v, Num n) => Subdiagram b v n m -> Point v n
location (Subdiagram _ a) = transform (transfFromAnnot a) origin

@danoneata danoneata mentioned this issue Oct 7, 2022
6 tasks
@danoneata
Copy link
Collaborator

Thanks a lot for the outline! That was super helpful!

I was also confused by the behaviour in that particular example, but you were of course right it was related to the local origin of the diagram. Works as expected after implementing the suggested changes. I'll move on to the next features.

This was referenced Oct 20, 2022
@srush
Copy link
Collaborator Author

srush commented Oct 31, 2022

Random project that uses the new with_names and subdiagram features.

https://mobile.twitter.com/srush_nlp/status/1587143608121057285

@danoneata
Copy link
Collaborator

As always, an amazing project! 🙂 Very addictive puzzles and beautiful illustrations! Well done!

In terms of the drawing API, as far as I can tell you had to resort to the more low-level primitives (that is, get_subdiagram), so do let me know if there is some higher-level functions that we should add to the library; maybe some functions for drawing bounding rectangles and frames?

@srush
Copy link
Collaborator Author

srush commented Nov 1, 2022

Oh good question. Let me think:

  • Definitely bounding rectangles and frames
  • Rendering speed for lots of components (maybe that means faster compositing)
  • Names on nodes of paths (ideally could name each of the points on the graph)
  • Constant-time lookup for names (I was scared to use names everywhere because I was a bit worried it would be quadratic to draw lots of connections)
  • A friendlier "transformation" object (I found I wanted to do work in one coordinate space and then transform it to another, but also still known the offsets. Think that could be better).

Crazier ideas:

  • Render arrows in between two already plotted values in Z-space (Not sure if this is possible in a functional system)
  • Support for working with numpy for batch plotting paths and connections

@danoneata
Copy link
Collaborator

Thanks for sharing these ideas!

  • Constant-time lookup for names (I was scared to use names everywhere because I was a bit worried it would be quadratic to draw lots of connections)

Yes, I've seen that diagrams caches a SubMap (a hash map from a name to its associated subdiagram), probably for exactly this reason. For ease of implementation I've chosen the more inefficient route (of traversing the tree when querying), but we should probably move to an approach similar to theirs.

  • Render arrows in between two already plotted values in Z-space

Forgive my ignorance, but what does "Z-space" mean here?

@srush
Copy link
Collaborator Author

srush commented Nov 2, 2022

Oh just like "order" - what is in front and behind.

https://en.m.wikipedia.org/wiki/Z-order

@danoneata
Copy link
Collaborator

Got it, thanks!

@danoneata
Copy link
Collaborator

One more potential enhancement idea from Sasha:

  • Replace transform with numpy generally... That would allow very fast scaling of trails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants