-
Notifications
You must be signed in to change notification settings - Fork 37
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
Paths with n segments #3842
Comments
We should be keeping things as close as possible to a standard mechanical engineer's code familiarity as possible which would limit solutions to for/if. I'm following the desire of reduce but I find it very rare to have CAD users have any familiarity with concepts from Java etc. Similarly we should avoid recursion at this point as perhaps there are perhaps some scenarios where it could be useful like fractal CAD that we've seen, but that is a niche use case and most likely not commercially applicable. If we can spare the user the overhead of coding in the logic we should. Recursive infinite loops also risks infinite engine overhead and I don't believe we have a way to flag for that right now which would be an unwelcome addition. I'm supportive of the two pre-built functions you highlighted, with a request for an optional argument for polygon() to draw only a subset of the polygon sides. For example a function call where you want five out of the eight sides of a polygon drawn. Not a common case but there will be situations where a tag is required for one edge/vertex of a polygon. Do your solutions support that? For example filleting one corner of a five sided polygon. |
I'll keep my feedback short since it's too easy to really go off the deep end anything language related. First off excellent proposal. You missed one solution though: generalizing our patterns to path segments. For n-gon, a polar/radial/circular pattern is sufficient.
Combine this with also We need examples where none of our pattern functions work. I'm with Jordan that we need to stick to ME code familiarity. I have my own opinions about where are achieving this goal and won't bring it up here. I will just say I agree for/if is "more familiar". Recursion is iteration is recursion - either works for fractal-like designs so it's not a problem but thanks for thinking about it Jordan :D. For tagging a single side or whatever the user can do I think @jtran would agree that KCL is past the point of being considered "immutable" - there are several functions that mutate state somewhere and have side-effects. I agree it'd be nice to keep immutability where possible though for easier reasoning, but we should do this on a case by case basis instead of saying "to keep KCL immutable" when it's already not. I can't see immutability being strong reasons for reduce or recursion. Note: for loop can also have restricted iteration so it's equally as restricted as reduce. |
Why not for loopsMy objection to
Here, non-loop designsJordan I agree that "we should be keeping things as close as possible to a standard mechanical engineer's code familiarity as possible", and that CAD users shouldn't be expected to use That's why I think we should offer a low-level and high-level way to get things done. The high-level way is I really strongly believe that power users should have access to the same tools as the people implementing the stdlib. If a power user runs into something they can't solve with polygon or repeatPaths, they should be able to use reduce or manual recursion to get the job done. Then they can design their own high-level API for the other people at their company, or publish it on the internet for everyone else to use. Or we can build it into the stdlib eventually. |
One way to think about it is we shouldn't nerf the language just because one persona (ME who's never programmed before) can't handle it. It may inform our priorities, but there are other personas we're designing for who absolutely need the lower-level tool. WRT mutations, @lf94, there's an issue for that that's on my radar. I hope to remove the mutation that we have. |
Wrt tagging some faces of a polygon, we could take an optional map of (side number -> tag) e.g.
|
My proposal: const newSketch = for i in 0..n, use outerSketch as innerLoopVariable {
// do stuff
continue innerLoopVariable |> line ([outerX, outerY], %)
} Equivalent to: const newSketch = [0..n].reduce((i, innerLoopVariable) => {
// do stuff
return innerLoopVariable |> line ([outerX, outerY], %)
}, outerSketch) |
Yes, I think the above could work. There might be a way to make it even more concise in the common case. For example, could the const newSketch = for i in 0..n {
// do stuff
continue |> line([outerX, outerY], %)
} |
Yes I think Maybe instead const newSketch = for i in 0..n use outerSketch {
// do stuff
continue line([outerX, outerY], %)
} (Removed the superfluous comma also) |
I'm going to get to work on |
@lf94, yes, I forgot the initial value. Thank you for pointing that out. The only nitpick I have is that (Aside and implementation note: I actually really like using |
Yes you're right |
I like the proposal Adam. I strongly agree with not wanting to introduce mutation, I think that's a gennie we won't be able to put back in the box later, and I think it's going to make introspecting harder, which is uniquely valuable to us. I agree that reduce is fine since it's not something every user needs to know about, but it's also a common enough pattern. I'm sure there's a way to do a If we're adding reduce, shouldn't we add map too? not sure of the implications but seems like those already familiar with reduce will expect map? can see reduce being hacked as a map, but I guess that kinda not possible since we don't have any array utils, maybe we should add a |
If we add This makes me think we should add namespaces to our stdlib, e.g. Geometry.TangentialArc.{to, relative, absolute} and Array.{reduce, map, push} |
Right now only arrays of literals work, e.g.
but we should allow
|
Adds an `arrayReduce` function to KCL stdlib. Right now, it can only reduce SketchGroup values because my implementation of higher-order KCL functions sucks. But we will generalize it in the future to be able to reduce any type. This simplifies sketching polygons, e.g. ``` fn decagon = (radius) => { let step = (1/10) * tau() let sketch = startSketchAt([ (cos(0) * radius), (sin(0) * radius), ]) return arrayReduce([1..10], sketch, (i, sg) => { let x = cos(step * i) * radius let y = sin(step * i) * radius return lineTo([x, y], sg) }) } ``` Part of #3842
If I followed the conversation correctly, and you want immutable push, then KCLs arrays cannot be backed by Rust It's possible. Clojure does it using something called hash array mapped tries (HAMT). But it also seems like a large change and increase in scope. The reason I came here was to post a link to a thread on functional for loops in Pipefish, which is very similar to what we came up with. The comments have a good critique. I particularly think the analogy to Sigma Σ summation notation in math is good. |
@jtran Funny, I also came back here with the idea of a Proposal
It enforces that the same identifier is used for outer assignment (i.e. result of the overall The above
Possible changes
More examples
@JordanNoone says he likes it. |
A few comments, but we are getting to the point of just needing to make a choice. We got some variant of a for loop. Good enough for the product.
|
Interesting. This design initially felt weird to me because of A couple questions:
This makes me want variable name reuse (shadowing) even more. |
Background
KCL's sketching API builds one path per function application. Sketching a square requires calling
line
four times:You can similarly make a pentagon with five
line
calls, a hexagon with six calls, etc.Problem
How do you write a function which draws an
n
-sided 2D shape, with somen
variable? Examples include:fn polygon(numSides)
(thanks Paul)fn zipTie(numTeeth)
(thanks Lee)parametric n-gon (a function which takes in
n: integer
and draws an n-sided polygon)? There's no way to call a function a dynamic number of times.You can define repetitive 3D solids using patterns, and you can repeat many individual 2D solids using patterns. But you cannot create a complicated single 2D sketch without very verbose repetitive code, and you can't make it dynamic.
More generally, KCL has no way to apply the same function a variable number of times.
Solution 1: For loops
The problem is this would require a new language construct (for loops) and mutation (you'd be mutating the sketch group to add a new segment to it each time). That's OK but if we can find a solution that keeps KCL's functional and immutable style, we should prefer that.
Solution 2: Recursion
This keeps KCL immutable and functional. It requires an
if
expression but that's fine we already plan to add that eventually (#3677). But recursion is a tricky concept and it can be easy to get wrong. If the user gets it wrong, their program loops forever, or crashes with a stack overflow. It feels a bit too low-level. Can't we give users a higher-level way to define this without using manual recursion?Solution 3: Reduce
The
reduce
function exists in JavaScript, Rust, all functional languages, and a lot of other ones too. For those who haven't seenreduce
before, it's basically a function which takes somestart
value and calls another function on itn
times. Reduce is basically this pseudocode:where
function
has the type signature(i: int, start: T) => T
. The output of onefunction
call becomes the input to the nextfunction
call (along with how many times the function has been called,i
). Until there's no more calls left.You could use it to call
line
functionn
times, each time adding a line to a sketchgroup.This keeps KCL functional and immutable, without users needing to write manual recursion. The reduce function is guaranteed to terminate, no stack overflow or infinite recursion. We don't even need to add an
if
expression to the language!Recommendation
We ship both
if
expressions and areduce
function. This lets users build their own dynamic solid2ds in a low-level (explicitly recursive) way or a high-level (reduce) way. These constructs might be unfamiliar and weird to MEs, but that's OK, they're aimed at power users. MEs shouldn't need to use them. Instead we should add these functions to the stdlib:polygon(numSides: uint, sideLength: float, center: point2d)
: draws an n-sided polygon whose sides are the given length, at the given center point.repeatPath(n: uint, f: (sketchgroup) => sketchgroup)
: given a closure which adds to a sketchgroup, repeat thatn
times. This would let Lee copy his ziptie bumpsn
times.This way MEs don't have to use recursion or reduce.
The text was updated successfully, but these errors were encountered: