Skip to content

Plan and priorities for KCL 1.0 #23

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Plan and priorities for KCL 1.0 #23

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0538a71
Draft plan and priorities for KCL 1.0
nrc Nov 19, 2024
7be798e
Add work on assemblies and caching
nrc Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 155 additions & 0 deletions one-point-oh.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
# 1.0 plan and priorities

We don't have time for big changes or big features (dev time + time to marinate). We know vaguely where we're going (see other design PRs).

High level priorities:

- Stability and polish
- bugs, performance, glitches, etc.
- User experience
- Goal: KCL is easy to read and comprehend
- Feedback is useful (though perhaps not pretty)
- IDE experience
- Error messages (parser in particular)
- Supporting core new features in engine, as required
- Future compatibility
- Not making tech debt worse
- And ideally a little bit better

Related:

- [Modelling app roadmap](https://github.com/orgs/KittyCAD/projects/35)
- [1.0 milestone](https://github.com/KittyCAD/modeling-app/milestone/2)
- [CAD primitives roadmap](https://github.com/KittyCAD/modeling-app/issues/729)

Priorities in **bold**, dependencies in *italics*, task size in t-shirt sizes (taking into account breaking change management; m-l means uncertainty about which, not 'somewhere in between').

## As required

### Bug fixing

**P0**:

- high priority bugs as they emerge
- plan to seek out and address lower-priority bugs during feature freeze


### Non-language feature work

Priority follows from other team's priorities.

New or more flexible std lib functions and code mods are expected as side effects of other work.

New features which might require significant work:

- [Multiple profiles in Sketches](https://github.com/KittyCAD/modeling-app/issues/1876)
- Assemblies?


### Pre-paying technical debt

**P1**:

- Don't make things worse by adding hacks, especially around the API of KCL

**P3**:

- Supplying user-side IDs to engine
- code mod API


## Language priorities
Copy link

Choose a reason for hiding this comment

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

I’d argue adding caching of objects for assemblies etc is a nice priority and making it fast which is more important than a few things on the list here

Copy link
Author

Choose a reason for hiding this comment

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

I'll add it in. I'll need to think a bit about how it can work, etc. My initial impression is that it will be a pretty large amount of work.

Are we aiming to get assemblies into 1.0? That was one thing I was unsure about for the feature work

Copy link

Choose a reason for hiding this comment

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

yes assemblies are required for v1, honestly i think we are like 50% of the way there for cache, we already have the hashes, we would need to only re-execute the objects that changed, there is already an api for object delete.

Copy link
Author

Choose a reason for hiding this comment

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

I added work items for assemblies and caching to the doc, needs elaboration though


**P1** (priority ordered):

- reserve keywords (s)
- https://github.com/KittyCAD/modeling-app/issues/4486
- and give meaning to underscore in identifiers
- function args
- named and optional args (m; *$0*)
- only one non-named arg (m; *$1*)
- ideally we make it the first arg, but doesn't have to be
- could require it be called `self`, but I don't think we need to and that makes back-compat harder
- simple tidying changes
- automatic conversion of round floats to ints, remove the `int()` function (s)
Comment on lines +68 to +69
Copy link

Choose a reason for hiding this comment

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

As mentioned on the thread in slack I don’t think we should remove int

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I forgot to respond on Slack. My thinking is that although it is a pretty small thing, it is one of many small things that all together make KCL feel a bit rough, and while not needing int alone isn't much, if we can polish all or most of these rough edges, then KCL will feel a lot nicer, especially for new users.

There was also a lot of discussion about other numeric stuff (all of which I think we should postpone to after 1.0). To be clear, I think not requiring int is a really small amount of work and that it doesn't make things any harder for the other numeric stuff, so IMO the cost/benefit here is good.

Copy link

Choose a reason for hiding this comment

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

okay i just want to make sure we aren't creating magic, do other programming languages do this automatic conversion of floats to ints. MEs aren't so dense as to not know the difference and I think giving them the control to change the type is better than not, BUT i guess if we are only talking about changing 10.0000000000000000000... to a int then its fine, but if we are talking about changing 10.000000000000000001 i say no

Copy link
Author

Choose a reason for hiding this comment

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

Could you say why you don't like the 10.000000000000000001 thing? We could definitely do just the former and it would get us most of the way there, but using a tiny epsilon would avoid some minor irritation for users.

It's not so much that it's a hard concept, just not one which fits with the rest of the language - we just have a number type, we don't ever expose the int vs not-int distinction explicitly, and it leads to really awkward behaviour like foo(5) working but x = 5; foo(5) being an error which requires x = 5; foo(int(5)).

Copy link
Author

Choose a reason for hiding this comment

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

Other languages do have an equivalence, e.g., Javascript has the position that all numbers are f64, but lets you use those in for loops, etc. (although their approach is to just accept floats everywhere rather than having an implicit conversion, I believe the conversion is better since for i in [0..(0.1 + 0.2) * 10] should either be an error or treat the upper bound as 3, whereas in JS the upper bound is equivalent to being treated like 4).

Example for why we might want a tiny epsilon is (0.1 + 0.2) * 10 comes out at 3.0000000000000004 and I would love to treat that like 3.

Copy link

Choose a reason for hiding this comment

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

I think with the precision we need for CAD I do not feel comfortable converting anything that is not 00000000 all zeroes without the user explicity saying to do so. Because for like a engine or a rocket they might actually want that.

- record init: `:` -> `=` (s)
- function decl: remove `=` and `=>`, `:` for type annotations (s)
- return type syntax?
- convert special strings (e.g., `END`, `XY`) to constants (s)
- std lib
- organise into modules (for docs, at least) (l)
- potential issues: namespacing, collisions, receivers, name shadowing for std
- ensure consistency, remove unnecessary or uncertain functions, check for anything we're badly missing (m)
- *depends somewhat on $3, we should do this in any case, but how we do it will depend on whether $3 is achieved*
- support optionally-implicit `%` in pipelines (m; *$2; depends on $1*)


**P2** (priority ordered):

- std lib
- remove use of anon records (l; *depends on $0*)
- immutable model
- well-defined rules for when engine calls cause a rendered object, add `construction` flag where necessary (m-l)
- tagging
- support `as` in pipelines (m)
- remove `$` syntax (m)
- where std lib functions take tags, ensure they're unambiguous and work with `as` variables (m)


**P3** (priority ordered):

- std lib
- reduce number of functions by using optional args (m-l; *$3; depends on $0*)
- `var`/`roughly` syntax for unconstrained points/numbers (m-l)
- changes how we do 'fixed point' constraints

## User feedback

TODO I don't really know what needs doing here, how it should be prioritised, or how it should be prioritised relative to the above language work.

- Error messages
- Parser
- IDE functionality
- Suggestions
- ...

## Docs and education material

Planned for feature freeze period (**P3** until then). We should do some planning before-hand (**P0**).

- std docs
- guide
- reference
- videos and other tutorial stuff


## Open questions

- Tags (these might be things we want to change for 1.0), in particular the current approach of collecting all tags into a `tags` field and allowing users of the geometry to refer to internal tags seems sub-optimal:
- source names are reified at runtime
- construction (including tag names) of an object should be private by default
- having a `tags` field violates the principle of tags being just variables
- a more principled way to do it is `export` of variables, but this 'works' for block syntax sketches, not so much for pipelines, and not for deeply nested tags
Comment on lines +202 to +203
Copy link

Choose a reason for hiding this comment

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

a more principled way to do it is export of variables

It's true that $foo binds foo locally, like an output parameter. But the .tags field is a whole separate use for tags.

A common use case is having a function that builds a sketch and returns it. If it were a matter of binding variables in a weird way that were accessible in the calling scope, you couldn't call the function multiple times.

Here's an example of this pattern that works today that's quite useful:

fn buildShape = (plane, offset) => {
  return startSketchOn(plane)
    |> ...
    |> line([1, 2], %, $important)
    |> ...
    |> close(%)
}

one = buildShape('XZ', 0)
two = buildShape('XY', 22)

extrude(10, one)
extrude(10, two)

// Sketch on each face.
buildMore(one.tags.important)
buildMore(two.tags.important)

When I first joined, I thought we wanted to hide the information. That may be true for assemblies. But for a single part, the user needs to be able to access everything in the scene that they could possibly click on.

As it is today, I don't see how it could work with static typing. The tags available in .tags would need to be part of the return type of buildShape().

On the other hand, if it were reified at runtime, it could look like this and be a runtime error if it's not defined:

buildMore(one.tags['important'])
buildMore(two.tags['important'])

A complication, which I think you're calling "dynamic tagging", is that even though Kurt et al have pushed for named tags for stability, others have brought up that if we have loops, then it doesn't really make sense to name a segment inside a loop. It needs an index corresponding to the loop index. So how does that work?

We used to declare the tags with strings, so people would actually do string concatenation between a name and a numeric index to declare them. This is all very dynamic.

Unless you have an idea for a completely different way of doing this, I think it needs to be dynamic.

Copy link
Author

Choose a reason for hiding this comment

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

So, I actually think there is a lot that is weird about this way of doing things, the dynamic-ness is the least bad bit. The fact that the name important is significant at runtime is weird because it makes variables significant which limits optimisation (or at least forces optimisation to maintain this info), it breaks encapsulation (because if the author changes the name of important, it break users of the sketch, which can't be caught statically in all cases because of indexing fields by non-literal strings), furthermore, if the structure of the final solid changes, then the meaning of important can change. The fact that important tags a line but is used to refer to a surface means that tags are ambiguous and I think this will be hard for users to understand in some cases.

I think there isn't a single solution, but multiple: naming and explicitly making some tags public is part of the solution, and I think these should become fields on the created sketch (so the equivalent of simply one.important, though not the extruded solid, IMO). It should also be possible to navigate to each component (or at least collection of components), so one.extrudedFaces would give you the set of faces extruded from the sketch. Finally, I think we should have a query system for finding geometry which is the general solution for finding any component which can be clicked, in this case, something like select(partOf = one, extrudedFrom = one.extrusionSketch.important). Obviously, this isn't a 1.0 thing though

- Any other precautions we should take for future compatibility? Including (but not limited to):
- higher order functions
- used in `patternTransform` pattern function, `reduce` and `map`
- are we happy committing to supporting higher order functions?
- how would they fit into a type system?
- Units of Measure
- syntax (is `num id` valid syntax today?)
- modules with units? (shouldn't be an issue since we only support imports within a project)
- Are there things we want to 'tighten up'?
- Indexing records (`foo['fieldName']`)
- Implicit conversion from array to record (pretty sure this is unintentional and should not work)
- Make arrays uni-typed?
- `KclValue` top type? (IMO this is the internals of the implementation leaking into the surface syntax and should be removed)


## Postponed

- [over-eager mutation](https://github.com/KittyCAD/modeling-app/issues/2728)
- Performance or tech-debt work
- point syntax (perhaps just change from `[]` to `()`? Or require `pt`/`vec`?)
- [construction geometry](https://github.com/KittyCAD/modeling-app/issues/1553), implicit or explicit `show`
- extensions to tagging
- 'dynamic tagging', e.g., for edges of a `polygon`