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

support anglebracket ⟨ ⟩, floorbracket ⌊ ⌋, and ceilbracket ⌈ ⌉ #27697

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

innerlee
Copy link
Contributor

Note: I know nothing about lisp, just start learning it three days ago. This is done by guessing. So feel free to comment or abandon or rewrite.

On this pr:

julia>1,22

julia> ⟨[1,2],[1,2.]⟩
5.0

julia>1.21

julia>1.22

@MasonProtter
Copy link
Contributor

MasonProtter commented Jun 20, 2018

I wonder if we should do something other than

⟨a, b⟩ = a * b

and instead maybe something like

⟨a, b⟩ = innerproduct(a,b)

where innerproduct falls back on * but may be overloaded. I don't know if that would actually have any advantages over just using * though.

@@ -2360,6 +2360,18 @@
((comprehension) `(braces ,@(cdr vex)))
(else `(bracescat ,@(cdr vex))))))))

((eqv? t #\⟨ )
(take-token s)
`(call anglebracket ,@(parse-call-arglist s #\⟩)))
Copy link
Member

Choose a reason for hiding this comment

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

The way we'd usually do this is to parse it as (anglebracket ...), so that it can be distinguished from the user writing anglebracket(...). Then add a lowering rule in julia-syntax.scm to convert it to a call. See for example how vect is handled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay

@JeffBezanson
Copy link
Member

I think the floor and ceiling brackets should just directly call (top floor) and (top ceil).

I'm less sure what to do with angle brackets; do we want them to only ever mean dot product?

@stevengj
Copy link
Member

I'm less sure what to do with angle brackets; do we want them to only ever mean dot product?

Single-argument angle brackets ⟨x⟩ are used to denote averages in some fields.

@@ -159,7 +159,7 @@
(define reserved-word? (Set reserved-words))

(define closing-token?
(let ((closer? (Set '(else elseif catch finally #\, #\) #\] #\} #\;))))
(let ((closer? (Set '(else elseif catch finally #\, #\) #\] #\} #\⟩ #\⌋ #\⌉ #\;))))
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this construct a Set object every time closing-token? is called? It seems like for performance we may want to define some global sets for this function.

Copy link
Member

Choose a reason for hiding this comment

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

If you look closely, the let is outside the lambda.

@@ -905,3 +905,7 @@ julia> map(splat(+), zip(1:3,4:6))
```
"""
splat(f) = args->f(args...)

anglebracket(x::Real, y::Real) = x * y
Copy link
Member

Choose a reason for hiding this comment

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

isn't this redundant with the dot method? Shouldn't this be defined in the LinearAlgebra package only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we leave some sort of dummy definition in Base? My vague impression is, if there is no anglebracket in Base, and PkgA and PkgB both defined and exported it, then there will be some kind of namespace clash? How to do it correctly?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can just do
function anglebracket end
which creates a function without any methods,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool!

@stevengj stevengj added the parser Language parsing and surface syntax label Jun 20, 2018
@MasonProtter
Copy link
Contributor

Another thing to think about is that it would be nice if this were designed in such a way to allow for bras and kets as discussed in https://discourse.julialang.org/t/a-qubit-dsl/8599/26

@ExpandingMan
Copy link
Contributor

Would it make sense to make the functions the brackets get lowered to undefined by default, so that e.g. anglebracket has to be defined by the user? (Having a default definition makes more sense for the floor and ceiling brackets.)

@MasonProtter
Copy link
Contributor

MasonProtter commented Jun 20, 2018

Would it make sense to make the functions the brackets get lowered to undefined by default, so that e.g. anglebracket has to be defined by the user? (Having a default definition makes more sense for the floor and ceiling brackets.)

For instance, some may want anglebracket(a,b) to lower to dot(a,b) whereas others would want it to be dot(conj(a),b) or dot(a, conj(b)), etc. Really all we need is enough parser support so that users can do the rest.

@ararslan
Copy link
Member

I agree, I think we should put in place a way to parse them and leave it at that for now, as we've done with many other Unicode symbols. Meaning can be added later or can be left up to packages.

@ExpandingMan
Copy link
Contributor

Also that's a good point, the angle brackets should most definitely not get lowered to multiplication by default, as it would imply some sort of conjugation in the complex case. But yes, it's probably best if it's undefined.

@innerlee
Copy link
Contributor Author

Should we add ‖ ‖ in the same way? I experimented a little bit and found it just works...

julia>1,-1-1

julia>1*‖⟨1,-1⟩‖
1

@StefanKarpinski
Copy link
Member

The lack of nesting is a bit of an issue but perhaps it could be controlled by unbalanced parens, e.g. ‖(x + ‖y‖)‖—you know the second doesn't close the first one because the parens aren't closed.

@innerlee
Copy link
Contributor Author

@JeffBezanson I "refactor" it using lowering rule in julia-syntax.scm, but this time, I completely don't know what I'm doing. It's kind of second-order guessing... Please check if the result is what in your head :]

@innerlee
Copy link
Contributor Author

And there is a question. If we do not define vertbracket in Base, then how to use it without top-level scope error?

julia> vertbracket(x) = abs(x)
vertbracket (generic function with 1 method)

julia>-1‖
ERROR: UndefVarError: vertbracket not defined
Stacktrace:
 [1] top-level scope at none:0

@StefanKarpinski
Copy link
Member

The ones that are uncontroversial: floor and ceil brackets can just lower to floor and ceil calls. It would be cool if the could be used for norm without allowing nesting using the rule that odd open a pair and even close the preceding odd one. The unclosed parens/bracked/whatever technique could be implemented later since those would be syntax errors with the alternating rule.

@StefanKarpinski
Copy link
Member

I know that ⟨ ⟩ are probably one of the prime motivators here but they're harder because you have to make a choice about how to lower them, which requires bikeshedding/design. If we have a dozen kinds of brackets, do they each get a new syntactic form? That seems excessive.

@innerlee
Copy link
Contributor Author

I checked the grand table of LaTeX symbols, the total number of brackets is very limited :P

@StefanKarpinski
Copy link
Member

total number of brackets is very limited

The total number of syntactic forms should be more limited. It's already probably too big.

We can change once consensus is reached.
@innerlee
Copy link
Contributor Author

innerlee commented Jun 21, 2018

Okay, I will wait for consensus or decision.

(but interestingly:

julia> x = 1.2
1.2

julia> y = -3.4
-3.4

julia> ‖x + ‖y‖‖
4.6

julia> ‖‖y‖+x‖
ERROR: syntax: extra token "" after end of expression

)

@JeffBezanson
Copy link
Member

vertbracket is really a totally separate matter and will require far more thought and design. There will be surprises and corner cases. Frankly I'm not sure we want to have it at all. Several places in the parser rely on being able to identify a "closing" token, and this would be the first case of a sometimes-closing token.

If you want to get any of these changes in, I strongly recommend putting them in separate PRs, since otherwise debate on one part will block all of it.

@innerlee
Copy link
Contributor Author

Separate them is easy, but how to do it right? i.e., do not give them pre-defined meaning in Base, and without the top-level error?

ERROR: UndefVarError: vertbracket not defined
Stacktrace:
 [1] top-level scope at none:0

@oxinabox
Copy link
Contributor

Right now you have:
⟨1, 2⟩ parses to anglebracket(1,2).
Do you have the parsing to anglebracketcat to match?

(expressions here are not quiet julia functions. they are what you get out of dump ing the expressions.)

Note that {1,2} parsing to braces(1,2)
and {1; 2} parsing to bracescat(1, 2)
and {1 2} parsing to bracescat(row(1, 2))
and {1 2; 3 4} parsing to bracescat(row(1, 2), row(3,4))

Also
[1,2] lowers to vect(1,2) (so similar)
and {1; 2} parsing to vcat(1, 2)
and {1 2} parsing to hcat(1, 2) (I feel like this should be vcat(row(1,2)) but it isn't)
and [1 2; 3 4] parsing to vcat(row(1, 2), row(3,4))

I think it is good to support the cat forms, for consistency.
I know from experience overloading the square bracket forms is possible and useful (I've not tried the braces)
And the code is already there for braces and square brackets.


I want all the bracket types :-)
I guess there is probably a work around to defining more syntactic forms if I am understanding @StefanKarpinski 's issues rightly:

Like making:
⟨1, 2⟩ parse to circumfix(Val{Symbol("⟨")}(), 1, 2).
⟨1; 2⟩ parse to circumfixcat(Val{Symbol("⟨")}(), 1, 2).
and
{1, 2} parse to circumfix(Val{Symbol("{")}(), 1, 2).
{1; 2} parse to circumfixcat(Val{Symbol("{")}(), 1, 2).
etc.
It is a big ugly though.
`bracket{}


Beyond the scope of this PR:
It would be nice if they were supported in call position.
Like f[i] is parsed into getindex(f, i) is for square brackets.
it would be nice if f⟨1⟩ parsed into anglebracketcall(f, 1)
I think it might be possible to fake this with juxtaposition multiplication and a bunch of promotion and conversion rules.

@innerlee
Copy link
Contributor Author

Here is the plan:

I will make a separate pr on ⟨ ⟩. The aim is to be able to merge it. So I will keep the changes minimal:

  • just parse it to (anglebracket blah...) in julia-parser.scm and make it (call (top anglebracket) blah...) in julia-syntax.scm, as the implementation now. In addition, disallow assignment and semicolon in the bracket.
  • add tests, docs, news
  • do not implement ⟨1; 2⟩
  • do not implement ⟨1 2⟩
  • do not implement ⟨1 2; 3 4⟩
  • do not implement f⟨x⟩

The latter four can be added later if we really want them.
Once it is in good shape, I will copy the way it was done to other brackets.

@innerlee
Copy link
Contributor Author

I'm not sure whether we want support broadcast, like .⟨[1,2],[1,2]⟩ = [1,4]

@JeffBezanson
Copy link
Member

The expression (top vertbracket) means it will call Base.vertbracket. We use this to provide "hygiene" --- if the user has a variable called getindex it won't break a[i] expressions.

@ararslan
Copy link
Member

Having the floor and ceiling brackets lower to floor and ceil calls seems a little weird to me. I think often what you actually want when you call floor(x) (for example) is floor(Int, x). But then do the floor brackets implicitly floor to an Int? That also seems weird...

@innerlee innerlee closed this Jun 23, 2018
@oxinabox
Copy link
Contributor

oxinabox commented Jun 23, 2018

I believe convention is to leave the issue open til the PR is merged.
(Though this is technically a PR is still the point where the full issue is described)

@innerlee
Copy link
Contributor Author

I agree, will reopen after beta is out.
(so the form as a pr doesn't matter?)

@innerlee
Copy link
Contributor Author

innerlee commented Jun 8, 2019

I will pick this up this summer 🕳

@PallHaraldsson
Copy link
Contributor

This is all intriguing, but would need to be done for JuliaSyntax.jl... since flisp is going away. It's likely of no good use to merge (only) for flisp.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parser Language parsing and surface syntax
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants