Graph-based ANF IR

Still WIP, but I made a start with the IR. Some points:

The nodes have an object hierarchy so that you can use isinstance.
I imagine that we might have situations in which we want to serialize the IR (e.g. create protobufs) and such, so trying to keep it pretty minimal.
Edges are one-directional right now, since bidirectionality is only needed in some passes (and requires more bookkeeping when mutating the graph) I think it's best to try and keep def-use edges separate somehow
I was thinking we can hide all the debug information in a single dictionary. Debug information would include things such as human readable names of the variable and the Python AST node that created the node from the parser.

Edges are one-directional right now, since bidirectionality is only needed in some passes (and requires more bookkeeping when mutating the graph) I think it's best to try and keep def-use edges separate somehow

I disagree. Every replacement operation, even something like x + 0 -> x, requires looping through uses to replace pointers. Most optimizations replace subgraphs by other subgraphs, so I expect nearly every pass will need uses in some form. As for the bookkeeping, there is already bookkeeping code for this in master right now.

I was thinking we can hide all the debug information in a single dictionary. Debug information would include things such as human readable names of the variable and the Python AST node that created the node from the parser.

Sure, that works, although I think it'd be good to have a property that points to some of the information, such as the name, to make debugging a bit easier.

In another line of thought, I think mutating nodes directly ought to be discouraged: instead, there should be some representation for changes in a graph, and individual optimizations should return a list of changes that the master optimizer can apply atomically. This would also allow an optimizer to compare changes from various optimizers and see if there are conflicts for example, or a debugger could listen for changes and create a visualization. This is the approach I use in the master branch, although it could certainly be refined. It could be enforced by making inputs a property that returns a tuple.

there should be some representation for changes in a graph, and individual optimizations should return a list of changes that the master optimizer can apply atomically.

I agree, and we should also (if possible) keep track of which optimization created which variable or function, and if there is a "canonical" link between an object in the new graph and one that was created explicitly by the user.
This is extremely valuable information when trying to understand why and where error happens, and it was a pain to retrofit in Theano.

That should be part of the optimization framework though, not the IR, right?

That should be part of the optimization framework though, not the IR, right?

Indeed, if we do not put debug information in the IR nodes but in a global dictionary, like you propose.

I disagree. Every replacement operation, even something like x + 0 -> x, requires looping through uses to replace pointers. Most optimizations replace subgraphs by other subgraphs, so I expect nearly every pass will need uses in some form. As for the bookkeeping, there is already bookkeeping code for this in master right now.

@breuleux This might be slightly different for ANF, but for SSA-based representations the problem is that use-def edges are available during parsing, but def-use edges are not (e.g. if you write loop, which introduce cycles in the graph). However, during parsing you want to do CSE and CP already--as I mentioned in a separate comment, the sea-of-nodes paper mentions that they get significant speedups in compilation by reducing the graph size during parsing by doing this. Hence, the approach they take (and I took in the first prototype) is that only use-def edges are available during parsing, and there is a separate pass later on that adds def-use edges for all the passes that need those (which indeed is most global optimizations).
So to clarify, when I see "keep separate", I don't mean in a separate data structure. What I mean is that the IR should be possible to construct and manipulate without def-use edges, and that those edges can be added later on. What I did in the first prototype is to just have a uses attribute which is set to None until the def-use edges are added.

In another line of thought, I think mutating nodes directly ought to be discouraged: instead, there should be some representation for changes in a graph, and individual optimizations should return a list of changes that the master optimizer can apply atomically. This would also allow an optimizer to compare changes from various optimizers and see if there are conflicts for example, or a debugger could listen for changes and create a visualization. This is the approach I use in the master branch, although it could certainly be refined. It could be enforced by making inputs a property that returns a tuple.

I agree that directly mutating the objects should be discouraged, but should it be enforced? I can imagine a situation in which we want to write some new crazy optimization, which we didn't think of when writing this atomic application/conflict detection/debugger/visualization framework, requiring us to update all of the aforementioned in order to get anything done.
I'd rather see the possibility of mutating objects directly, but also having a set of functions which will apply optimizations in a safe way.

def-use "edges" aren't really edges, they are just bookkeeping to allow backward traversal of the graph, like a doubly-linked list. They are trivial to maintain during construction, since you just need to add back pointers every time you add an edge. The main pain point is that they interfere with dead code elimination: even if you disconnect part of the graph from the output, def-use edges will still point to it. This prevents the GC from claiming the dead code and adds unnecessary work to node replacements, since they might redirect dead links. I suppose that not having def-use edges during graph construction spares us that trouble (even though we will need to address this during optimization), and if we want to serialize the graph, we probably want to avoid storing them, which means we should have a pass that can reconstruct them from scratch regardless. So I'm okay with doing things as you propose.

Regarding mutation: by "enforcing" immutability I mostly mean making it difficult to do it accidentally, by locking down the inputs field. I don't have an objection to adding e.g. a node1.set_input_unsafe(i, node2) method.

def-use "edges" aren't really edges, they are just bookkeeping to allow backward traversal of the graph, like a doubly-linked list. They are trivial to maintain during construction, since you just need to add back pointers every time you add an edge. The main pain point is that they interfere with dead code elimination: even if you disconnect part of the graph from the output, def-use edges will still point to it. [...]

I am aware of all of the above, but I am talking about the case during parsing e.g. as I am parsing y = 1; f(y); g(y), the use-def edges at any given time are correct, but the def-use edges of y won't be correct until I have completed parsing the full program.

I would like to discuss adding an Edge type, for the following purposes:

The uses set of a node needs to store pointers to predecessors as well as which of the predecessors' inputs point to the node. Edge could pack that information together.
It makes sense to store debug information about edges: if some parameter x is used six times, there is still only one x node, but six edges pointing to it. The location in source code of each use can't properly be stored in the node, but it can be stored in the edges. Furthermore, if an optimization replaces node A by node B, it does so by replacing edges, and that's a good place to track what the optimization did.

One way to do this would be with an Edge(node1, edge_type, node2) class. node1.inputs would contain a list of Edge instead of Node, which would unfortunately add some indirection. node2.users would contain a set of Edge which is what we want. Alternatively, we could have node1.inputs and node1.inputs_debug as two separate lists. Or all that information could be stored externally, I don't know.

I'm going to push back on this idea a bit, because I think it's a large complication of the data structure with questionable benefits. I think we should keep the IR as simple as possible. Partly this is because of robustness, but also because of readability: I can reason easily about z = x + y producing a node z that has two inputs, which are nodes x and y, all that indirection in there is too much cognitive overhead for a central data structure.

I don't think the input index needs to be stored separately at the definition node, since it is easily available by walking the list of inputs of the use node. This logic can be put into a method once, and never looked at again. On the other hand, saving it separately introduces redundancy and indirections. If on the long term we run into performance issues, we can either revisit this, or the optimization pass can store some auxiliary information itself.
A variable is only created once i.e. there is one point in the code where y = f(x). The node representing y contains the source code location of this definition. If y is used 6 times, e.g. in z = g(y), then each of those locations is stored in the node representing its use (e.g. z). Every node will have exactly one corresponding source location. The problem does occur for constants, as you mentioned before, but I found it doubtful that the Edge type should be introduced for this case (in most cases you can look at the uses of the constant to figure out what the corresponding source code literal is).

I don't feel too strongly about having the uses attribute being a set of tuples, the Edge thing I'm more hesitant about though.

On my phone, so short answer: Every node has a graph attribute, so if
is_ouput is true, you can just look at that.
On Thu, Dec 14, 2017, 15:21 Olivier Breuleux<EMAIL_ADDRESS>wrote:

@breuleux commented on this pull request.
In myia/ir.py
https://github.com/mila-udem/myia/pull/3#discussion_r157051749:

@@ -49,22 +53,23 @@ class Node:
def init(self) -> None:
"""Construct a node."""
self.inputs: List[Node] = []

   self.value: LiteralType = None

   self.uses: Optional[List[Node]] = None

The set of tuples approach introduces redundant information as well e.g.
if I change the order of inputs I must be sure to update the inputs' uses
attributes to reflect this.
The interface you should be using to change inputs should do this for you.
Tuples or not, handling the bookkeeping for uses yourself instead of going
through a safe API is a recipe for disaster.
Example of where you don't need the indices: I want to find all the
addition nodes where I am an input, and the other input is a scalar, and I
don't care about whether I am the first or second input, since addition is
commutative.
I mean, the index does help a little in this case, because it gives you
the index of the other input directly:
for pred in node.uses:
if isadd(pred.inputs[0]) and any(constant(x) for x in pred.inputs[1:] if x != node):
...
vs
for i, pred in node.uses:
if isadd(pred.inputs[0]) and constant(pred.inputs[3-i]):
...
If there is a case where we need to identify a node as being an output
easily, why not just add a boolean is_output attribute?
You don't just want to identify it as an output, you want to identify the
graph it is an output of: if you replace A by B, and A is the output of
graph G, you need to change G's output pointer to point to B instead. I'll
also note that a node could be, in theory, the output of more than one
graph, but we probably want to merge them in that situation.
Since uses is supposed to be the reverse of inputs
I mean that's the point, I don't necessarily agree that this is all what
it is supposed to be. I view it more as a repository of what points to this
node and should be adjusted if this node is replaced.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
https://github.com/mila-udem/myia/pull/3#discussion_r157051749, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACMWy6i17Dp-9O7TRXXo4-36TJRKtt2sks5tAYM6gaJpZM4RBc45
.

On my phone, so short answer: Every node has a graph attribute, so if is_ouput is true, you can just look at that.

It's not that simple, unfortunately. Consider:
def f(x):
    a = x * 1
    def g():
        return a
    return g

a.graph is f, because that's where a is computed. It is, however, the output of g, a different graph entirely.

Then maybe we should revisit the idea of having a return node, which would
belong to g but have a as an input. That's a more principled approach than
edges sometimes being nodes and sometimes being functions, which requires
constant checks to see which one of the two it is.
On Thu, Dec 14, 2017, 16:24 Olivier Breuleux<EMAIL_ADDRESS>wrote:

On my phone, so short answer: Every node has a graph attribute, so if
is_ouput is true, you can just look at that.
It's not that simple, unfortunately. Consider:
def f(x):
a = x * 1
def g():
return a
return g
a.graph is f, because that's where a is computed. It is, however, the
output of g, a different graph entirely.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
https://github.com/mila-udem/myia/pull/3#issuecomment-351840187, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACMWy8CNKwanTG1FbAsZhsPOdCP5fR_Vks5tAZIdgaJpZM4RBc45
.

Yeah, Return nodes would work -- single input, belong to their graph, can never be replaced. Good call.

I still think it is valuable to keep everything simple. That we can just
hide all the complexity behind helper functions is unrealistic. Things will
break or need to be changed, and when they do, simplicity will be a boon. I
personally much prefer transparency; it reduces the startup cost (no need
to read API docs for simple operations); it simplifies debugging (the less
abstraction the easier it is to understand what is going on); it simplifies
e.g. serialization; etc.
Happy to change to sets of tuples. I agree with your last argument that
it's a better one-to-one correspondence to edges.
I don't think that we should modify our representation just so that we can
carry over a full history of the graph. This information should be put
somewhere else (e.g. the debug attribute can just have a dictionary indexed
by its inputs with this information). It's too much added complexity for
information that is only for debugging.
On Thu, Dec 14, 2017, 16:00 Olivier Breuleux<EMAIL_ADDRESS>wrote:

I can reason easily about z = x + y producing a node z that has two
inputs, which are nodes x and y, all that indirection in there is too
much cognitive overhead for a central data structure.
A property can eliminate the indirection, or we could store the edge debug
information in a separate list.
In either case, it's not clear to me that we'd often deal with the data
structure directly. Anything that requires matching a sub-graph is, I
think, best expressed declaratively, iterating over nodes should be
abstracted away, and so on.
I don't think the input index needs to be stored separately at the
definition node, since it is easily available by walking the list of inputs
of the use node. This logic can be put into a method once, and never looked
at again. On the other hand, saving it separately introduces redundancy and
indirections.
Bookkeeping uses without the index is no less complicated than including
the index in the bookkeeping, and it is arguably easier to mess it up: if
A's first input is B and you replace it by C, can you remove A from B's
uses? Not if B figures in A's other inputs! (index, node), on the other
hand, always represents one and exactly one edge, it's very simple, you
can't mess this up. So I take exception to the idea that it's simpler or
easier not to store the input index. It's not.
If y is used 6 times, e.g. in z = g(y), then each of those locations is
stored in the node representing its use (e.g. z).
Optimizations complicate this. Suppose you have the following:
a = f(x)
b = f(x)
c = g(a, b)
b will be erased by CSE, so you will end up with c = g(a, a). Storing
debug information in edges lets us remember that the second a is, in
fact, b. It also lets us store the information that the edge to the
second a, but not the first, was drawn by CSE. You get much more fine
grained information this way.
(in most cases you can look at the uses of the constant to figure out what
the corresponding source code literal is).
Not all constants will be in source code. Some may be introduced by AD.
Others may be introduced by optimizations.
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
https://github.com/mila-udem/myia/pull/3#issuecomment-351834326, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ACMWy81Y-ET7qNLVm2BYQ73ciYETKm-Jks5tAYx9gaJpZM4RBc45
.

@breuleux In e6e8da181ee0818749b3c013ddd31583ae058a39 I added the doubly linked graph by default as you suggested. I wanted to avoid a custom API though (set_input(i, n)/set_input_unsafe(i, n)), so I created a custom MutableSequence whose only job it is to update uses.
This allows you to add bidirectional edges using node.inputs[2] = n (including append, extend, insert, del, etc.) and you can add unidirectional ones by modifying node.inputs.data.

Also, from discussing with @breuleux we're not sure that we want to allow arbitrary mutation of the inputs.  We should probably only allow to replace an input for optimization purposes.  Anything more complex should create a new node and replace the old one.

@abergeron When converting the AST into ANF, I found it more natural to progressively add inputs in some places e.g. when constructing the function for a loop body you don't know in advance how many parameters there will be.
Second question is slightly higher level: Considering we need to have the ability to replace inputs, what safety do we get from not allowing other mutations? Also, aren't we just moving the goal posts: We might be doing fewer "complex" mutations on the nodes themselves, but we are actually doing more complex mutations on the graph overall i.e. we are replacing nodes instead of just rewiring them, which means e.g. a program can have a reference to a node that is no longer part of the graph.
Lastly: Could you clarify what practical changes that would mean for the code in this PR?  I think avoiding mutability when possible is a good habit to have. On the other hand, I am not sure that good habits should necessarily be strictly enforced. We can have our algorithms create new nodes throughout the framework, without writing overly defensive code in the IR itself that completely disallows mutating in case we need it later/somewhere else/for a quick hack/etc.

@breuleux You mentioned in another PR that we are "not close to finalizing the IR". Could you clarify what things you would like to see added or changed? The two open topics of discussion I see:

Your proposal to represent edges using their own objects so that we can attach debugging information to them. I'm not convinced that this extra level of indirection is justified if it is only to make carrying around debugging information easier. If there is a specific use case you have in mind, I am happy to try and compare the two versions to see how they compare.
How strongly should we enforce mutability? I am in favor of as much immutability as possible, but the inputs of a node have to be mutable, because replacing a node with another requires changing the inputs of connected nodes. Instead of allowing this but enforcing restrictions on other forms of mutation through overly defensive programming, I am more inclined to rely on documentation and guidelines which discourage mutating nodes instead.

If this IR looks okay in its overall design, I can start adding unit tests and start slowly working towards parsing functions by combining #4 with the work that you did in master.

There are several issues about the IR I have not mentioned yet. I discussed them a bit with @abergeron this afternoon and I will write them out here.

Keyword arguments. The current IR only supports ordered arguments, but supporting keyword arguments isn't a whole lot more complicated and it is a nice convenience for the user. I suppose they can also be eliminated through relatively simple analysis, if it comes to that.

Varargs are also a big convenience -- they allow a generic implementation of grad from J, for example. How do we support these?

The print statement is entirely unusable with this IR because it will always be dead code. Is this what we want?

More generally, this IR completely erases ordering information. This seems foolhardy to me -- sure, we want to erase it eventually, but certainly there's a few ordering-aware passes we could run early to improve user experience. For example: with this IR the user will essentially have to handle the RNG state explicitly, e.g. R, x = R.randn((10, 10)). But we could also allow x = R.randn((10, 10)), preserve ordering information, and then transform the calls automatically. Given that we can statically know what R.randn is, that seems doable.

A tentative proposal to take care of these issues would be to make inputs a dictionary and use a special keyword-only argument called __prev__ to every call to point to the previous call in the user's code, which lets us preserve ordering and "dead" calls like print (a poor man's monad, I guess). Then, we run inference on this, we identify all ordering-sensitive functions, and we drop all ordering constraints except those that link these functions.

I feel like this was discussed at length at one point, but I can't find the discussion right now. Maybe it was from before you joined? Either way, it's worth picking this apart a bit just for clarity sake: The print statement and the RNGs are problematic for the same reason, which is that they rely on state. This represents a problem in the IR, and a problem in the surface language.
In the IR, I believe we should try to use a principled approach to dealing with state, because we might need it later if we decide to introduce e.g. memory operations, BLAS handles, networking/NCCL primitives, etc. This probably means having a monad/state object which is passed around. This is what e.g. Haskell and sea-of-nodes do as well for memory and I/O.
From the user's perspective, there are two options:

Ask the user to carry around the state explicitly (feasible for an RNG, bit more awkward for print, but e.g. Haskell does this)
Use the ordering information to automatically introduce a state that is carried around in the IR.

Approach (2) is also what e.g. sea-of-nodes uses to deal with memory and I/O, for which it works fine. However, it prevents parallelism by introducing strict dependencies which is problematic if we use it for something computationally intensive like random number generation. If I write randn 10 times in a row, the sampling will happen sequentially instead of in parallel. If you don't care about reproducible random numbers--many people don't--this might not be acceptable Ideally, the user would have control over the reproducibility of their random numbers e.g. TensorFlow allows you to switch from non-deterministic to deterministic by setting a global seed which then implicitly gets passed around (I like the fact that the option is given, I'm not convinced it should be done implicitly though). Similar issues arise with asynchronous calls, where you don't want to enforce an order. The second problem is that some people want a reproducible set of random numbers, but one that changes at each function call. This means that we have to have some concept of a memory/RNG state that can be carried across function calls as well.
I am not entirely sure I follow your proposal of a __prev__ keyword argument. If it functions as a poor man's monad, would print and randn use the same one? It seems like a bad idea to introduce ordering constraints between functions that rely on different states, but maybe I am not understanding it correctly. My one rough outline would be:

We need some support for monads. These monads can be implicitly inserted using ordering information (e.g. for print function calls). They must be accepted and returned by each function.
It needs to be easy to introduce new monads, because we want separate ones for separate resources e.g. memory, I/O, RNGs, network.
We need to be able to execute some functions in a non-ordered way e.g. random number generation and network calls. These functions can simply be input-less functions.
Monads must be able to be carried over between function calls e.g. the memory and RNG state.

So one proposal is that there are two types of random calls: One of them is orderless, and can be readily parallelized. The second takes a memory monad as input and output, and reads its RNG state from there. This memory monad is an input and output of each function call, and is carried across top level function calls (so that subsequent calls can generate different numbers). The user can set the seed in this memory before a function call (i.e. outside of the Myia runtime) in order to make the numbers deterministic.
More concretely: We start with two monads, I/O and memory. In the runtime they simply take the form of a key-value store (a dict). I can try to flesh out the implicit insertion of these monads as part of the parser, which should be doable. Note that nothing needs to actually change about this IR; monads are inputs and outputs like any other.

I'm going to bed, so I'll think about the kwargs and varargs tomorrow. If at all possible though, I think it would be best to keep their support limited to either a separate, intermediate IR, or get rid of them in the parser already. The simpler we can keep the canonical IR the better. Can you elaborate on why varargs are needed for grad?

So we can probably support variadic functions by interpreting f(*args) as syntactic sugar for "pack arguments in a typed list". This adds the constraint that all the arguments must be of the same type, but this is the same constraint that Python's type hint system enforces. This way the IR nor the type inference needs to be changed. We can do the de-sugaring pass on the AST, or we can do it as part of the parser; that might be better than extending the IR with sugar.
For keyword arguments, we can rewrite the call f() of def f(a=2) to f(NOT_PASSED) and def f(a): a = 2 if a is NOT_PASSED else a. This desugaring can probably happen on the same level (AST or parser) so that we can keep the IR small. (Or maybe we should have a second IR which is an extension of this one, which supports a bit more Python magic?)

If I write randn 10 times in a row, the sampling will happen sequentially instead of in parallel. If you don't care about reproducible random numbers--many people don't--this might not be acceptable.

We can have a version of randn that's sequential, and another that imposes no ordering constraints. They would be easy for the user to swap. But I see you propose the same thing later.

I am not entirely sure I follow your proposal of a __prev__ keyword argument. If it functions as a poor man's monad, would print and randn use the same one? It seems like a bad idea to introduce ordering constraints between functions that rely on different states, but maybe I am not understanding it correctly.

"Monad" is probably not the right word, and it's the first idea that popped in my head, so it might not be particularly good. I'm saying have the parser process:
a = f(x)
g(x)
b = h(x)

as:
a = f(x)
_t = g(x, __prev__=a)
b = h(x, __prev__=_t)

This is prior to knowing what f, g and h refer to, and is meant to keep the ordering information through a chain that goes through the inputs. We would then run a dataflow analysis on that IR in order to figure out what calls correspond to what state, and then rewrite them monadically.
Alternatives:

A prev field in Node.
A list in Graph representing the ordering of the nodes in the source code. I think I actually prefer this idea.

I can try to flesh out the implicit insertion of these monads as part of the parser, which should be doable.

I don't think this is a good idea. You need type and value propagation to know which calls need to be processed: randn could be given as a function's parameter, for example. I think it is simpler to keep the ordering information as a list of nodes, then run a different pass to infer what needs to to be inferred, then insert the monads. This doesn't require changes to the IR either (besides one extra field in Graph) but it keeps the parser lean and simple.

If at all possible though, I think it would be best to keep their support limited to either a separate, intermediate IR, or get rid of them in the parser already. The simpler we can keep the canonical IR the better.

Is the IR simpler without kwargs? Passing a list of arguments isn't intrinsically simpler than passing a dictionary.

Can you elaborate on why varargs are needed for grad?

It needs to return a function that has the same signature as its input:
def grad(fn):
    jfn = J(fn)
    def g(*args):
        _, bprop = jfn(*map(J, args))
        return bprop(1.0)[1]
    return g

Another example would be compose:
def compose(f, g):
    def rval(*args):
        return f(g(*args))
    return rval

Basically, any higher order function that transforms one or more functions in a way that's generic to their input signature needs this.

So we can probably support variadic functions by interpreting f(*args) as syntactic sugar for "pack arguments in a typed list". This adds the constraint that all the arguments must be of the same type, but this is the same constraint that Python's type hint system enforces.

This will not work for the use cases above. What I have in mind is to treat them a bit more like macros and require that we can precisely infer what's in *args (number of arguments and their types) in every situation. I would rather not try to do this in the parser directly, though -- this is inference.

For keyword arguments, we can rewrite the call f() of def f(a=2) to f(NOT_PASSED) and def f(a): a = 2 if a is NOT_PASSED else a. This desugaring can probably happen on the same level (AST or parser) so that we can keep the IR small.

You mean optional arguments? I'd forgotten about those. Anyhow, adding NOT_PASSED to f's arguments requires inference, so it'd be best to do it after parsing.

(Or maybe we should have a second IR which is an extension of this one, which supports a bit more Python magic?)

Yes, perhaps we can simply add an ExtApply node. Extend the abstract interface for inputs to act as a dictionary, which doesn't really complicate it. Then we can let ExtApply use any key, but Apply only allows integers and is backed by a list. Additionally, ExtGraph's parameters list could contain Parameter, StarParameter and KeywordParameter, as well as a list of nodes that preserves ordering. We can then perform inference on ExtGraph and use the results to clean everything up, add monads, etc.

A list in Graph representing the ordering of the nodes in the source code. I think I actually prefer this idea.

Yes, I would prefer this approach as well. It's trivial to build during parsing, and a single call to graph.nodes.clear() turns it into an unordered graph again.

Is the IR simpler without kwargs? Passing a list of arguments isn't intrinsically simpler than passing a dictionary.

Conceptually it is perhaps not intrinsically simpler, but it is simpler by the virtue of an array being a simpler data structure than an associative array. A few practical things that come to mind:

This forces arguments to be named. User-defined functions come with parameter names, but new parameters introduced by IR transformations would require a "parameter name" generator that avoids naming conflicts. This shouldn't be too hard, but I can see this getting annoying.
Performance. I don't want to get too much into premature optimization, but case in point: The sea-of-nodes paper literally has a section devoted to how they use a pool to allocate nodes because that was causing too much overhead. We are expecting out graphs to be smaller (which is why we are taking the liberty to write our first version in Python), but making following an edge requiring a hash table lookup might be pushing it. We want to avoid compilation wait times like Theano and TensorFlow have.
Maintaining argument order might be useful when lowering. It might be confusing if we eventually lower to e.g. C++ and the arguments of a compiled function corresponding to a user function have changed order. Pretty minor though.
Serialization formats (Protocol Buffers, MessagePack) trivially deal with lists, but dictionaries will require some more consideration (saving them as two lists of keys and values, probably).

This will not work for the use cases above. What I have in mind is to treat them a bit more like macros and require that we can precisely infer what's in *args (number of arguments and their types) in every situation. I would rather not try to do this in the parser directly, though -- this is inference.

I guess the complexity of this depends on the restrictions we impose, because it sounds like it can get pretty tricky pretty quickly. Having variadic functions returned looks like it needs constant propagation at least, but having them passed as arguments to higher-order functions basically requires full-fledged abstract interpretation, no? Then there are some other drawbacks to macro expansion. For example, assuming sum is variadic this would blow up your compiler:
l = []
for i in range(10000):
    l.append(i)
    sum(*l)

The bigger question for me is: Do we really need this in version 0.1? If not, are variadic functions something that can be added later without requiring a fundamental rewrite? I would answer the first question with a resounding no (the grad operator can be hard coded instead of being expressed in terms of the J operator for now). The starting point of Myia is a machine learning framework with support for general control flow. We pushed that quite a bit already with support for higher-order functions and such. I don't think we should push this further to a language that tries to compete with Haskell in version 0.1. Let's keep type-safe variadic functions with arbitrarily typed arguments for future work, and let's focus on compiling an MLP first.

Yes, perhaps we can simply add an ExtApply node. Extend the abstract interface for inputs to act as a dictionary, which doesn't really complicate it. Then we can let ExtApply use any key, but Apply only allows integers and is backed by a list. Additionally, ExtGraph's parameters list could contain Parameter, StarParameter and KeywordParameter, as well as a list of nodes that preserves ordering. We can then perform inference on ExtGraph and use the results to clean everything up, add monads, etc.

I like this approach. It's in line with compiler architectures to have various IRs for different stages. Moreover, it gives us the modularity we want in development: We can start with this simple IR, and write the parser to directly generate this one. In a next step, we can add an extended IR alongside with a lowering pass to this IR, and we can update the parser to use this new IR in order to support more Python features.

So what's next for the IR in this PR? Shall I start adding tests? My current view is that we should go for deep instead of wide i.e. I'd rather move on to the parser and the optimization framework and get x + y working than focusing on advanced futures like variadic functions (that is, given that we don't see any fundamental obstacles to adding them in later).
The tl;dr from my perspective. Am I missing anything?

We can separate out support for keyword/optional arguments and argument packing/unpacking in a later stage by adding an extended IR that the parser can be adapted to use. For now, the parser can use this simple IR.
The optimization framework, which will be added later, will try to avoid mutating nodes whenever possible, and instead replace existing nodes with new ones. However, since we need to keep the inputs mutable, this will be a guideline rather than an enforced restriction in the IR itself.
I can add an optional attribute to Graph (or this can be an extended, separate IR) with an ordering of the nodes, which can be used by passes that require this information (i.e. implicitly adding monads to functions that deal with I/O and memory). This too can be done in a separate PR later on.

Now with tests and 100% coverage.

[Using a dictionary] forces arguments to be named.

It does not. Use the next available integer key.

Performance. I don't want to get too much into premature optimization, but case in point: The sea-of-nodes paper literally has a section devoted to how they use a pool to allocate nodes because that was causing too much overhead.

That wasn't clear enough in my post, but I'm talking more about interface than implementation, namely that the notion of non-integer keys should be "compatible" with the interface. If only ordered arguments are present, the implementation can be a list. However, we may want, in some situations, to allow a wider range of keys, and it costs little to design the interface to handle this gracefully.
As a meta-comment on this, what I am trying to do here is to hedge against undercomplexity. Sure, simplicity is nice, but it is trivial to come up with simple solutions to simple problems. What is much harder is to find simple solutions to complex problems, where we need to deal with varargs, keyword arguments, track debugging information, and so on.
My concern is that undercomplex solutions can be very difficult to adapt to add needed features or complexity, and are therefore prone to devolve into overcomplex solutions as people desperately try to hammer in square pegs in the round holes they made (I would argue that this happened to Theano). On the other hand, we don't want to be overcomplex right off the bat. So what I think is probably the sweet spot here is to design a system that we know can support all the complexity we might need, but can be downgraded gracefully. For that, we need to think as far ahead as possible and estimate the size of the hurdles before we get there.
I think having ExtApply and the like is probably good enough for all that -- we can represent the features, clean them out, and produce a simpler IR. However, I would nonetheless argue that if some functionality could easily be made to work on both Apply and ExtApply, it should be made to work on both. This would allow to e.g. reuse inference code and possibly some optimization code. Hence my push for a flexible interface. For example, arguments can be matched to parameters with zip if they are both lists, but that won't work on ExtApply/ExtGraph, even though the feature being implemented doesn't need to care about that detail. Abstracting this into, say, a match_parameters method on graphs would be better. These are the sort of details I want to keep in mind.

Having variadic functions returned looks like it needs constant propagation at least, but having them passed as arguments to higher-order functions basically requires full-fledged abstract interpretation, no?

It's just a kind of type inference. If you see a call to f(*args) where you know that f has type (a, b) -> c, you know that *args must have type (a, b); if g has type (a, *args) -> c and *args has type (a, b) you know that g has type (a, a, b) -> c, and so on. That would be enough to support grad, compose, and so on if their arguments are known statically. I don't propose to do anything more than that. I certainly don't propose to infer how many arguments to pass to a variadic sum function ;)

If not, are variadic functions something that can be added later without requiring a fundamental rewrite?

I didn't take variadic functions or keyword arguments into account at all on master and when at some point I started looking at a way to add support, I basically gave up. I think the new IR would fare better, but I'm not going to get burned twice on this. I reckon having a distinct ExtGraph with support for the bells and whistles will be good enough.

My current view is that we should go for deep instead of wide i.e. I'd rather move on to the parser and the optimization framework and get x + y working than focusing on advanced futures like variadic functions (that is, given that we don't see any fundamental obstacles to adding them in later).

That's all right, but I do want to plot the width extensively, which is what this discussion is for. The obstacles we can expect, most of the time, are not "fundamental" but a natural consequence of code growth around false assumptions that we didn't take proper care to fence. If we know in advance where advanced features are supposed to fit, we can work to maintain the spot and make sure it doesn't get overtaken by overgrowth.

However, since we need to keep the inputs mutable, this will be a guideline rather than an enforced restriction in the IR itself.

You've made that point a few times, but I'm confused. My suggestion was to put the functionality behind a set_input/set_input_unsafe method in order to add tedium, better signal what the guideline is (by making inputs read-only), and control/limit the interface (do you really need to append to this list?) It's not about making them literally impossible to mutate.

My suggestion was to put the functionality behind a set_input/set_input_unsafe method in order to add tedium [..]

Python has well-defined abstract interfaces for dealing with containers, and I would much prefer if we stick to those.

Over time, we might realize we want more complex behaviors in some corner case e.g. clear, pop, append, etc. We will have to implement a separate API call for each of these. By the end of this we will basically have replicated the MutableSequence interface that already exists. We get these methods for free by using the ABC.
Principle of least astonishment. If I see an inputs attribute which is a list, I expect to be able to use Python's constructs for mutating lists instead of having to use separate methods. I also know exactly how I can expect each of these constructs to behave. (Similarly, if inputs would be exposed as a tuple, I would not expect it to be mutable through set_input calls.)

It does not. Use the next available integer key.

This is what Lua does, and it's not pretty. What should append do when the keys are 0, 2 and 3? Or when the keys are -3 and -2? You can define behavior for all of these cases (Lua does), but it can get really awkward.

However, we may want, in some situations, to allow a wider range of keys, and it costs little to design the interface to handle this gracefully.

Python's container ABCs are this interface. If the need ever arises, we can extend Inputs to inherit from MutableMapping as well and make it work like a Lua table.

So what I think is probably the sweet spot here is to design a system that we know can support all the complexity we might need, but can be downgraded gracefully. For that, we need to think as far ahead as possible and estimate the size of the hurdles before we get there.

This is a bit meta and perhaps I am misinterpreting, but I would say that the sweet spot is to design a system that can be readily extended, not one that can be downgraded. YAGNI, KISS, MVP, DTSTTCPW, overengineering, worse is better... There are more mantras than I can keep track of warning developers to keep things as simple as possible until you are forced to add functionality.
So I would rephrase "a system that can support all the complexity we might need but that downgrades gracefully" to "a system that supports the minimum but which can readily be extended to support all the complexity we might need". Ensuring extensibility and maintainability is generally good practice, and there are plenty of guidelines to follow to ensure that we do this (SOLID, agile development, etc.).

That's all right, but I do want to plot the width extensively, which is what this discussion is for.

Agreed. I don't want to shut down the discussion, but if we agree that certain functionality can be added later, maybe we should move the discussion to a separate issue so that it is clear that this is not a blocking issue for merging this PR.

Python has well-defined abstract interfaces for dealing with containers, and I would much prefer if we stick to those.

But -- abstractly speaking -- what kind of container is the set of outgoing edges from a graph?
So, okay, think of it this way: we may very well get a node type that supports keyword arguments, if only in one of the beginning passes. But there are many operations we want to do generically on all node types, like checking reachability, matching inputs to parameters, replacing some edge, and so on. The code to do this using an inputs list will fail to generalize.
So here's an idea: remove inputs from Node -- only the Apply node will have this field. Add an edges or outgoing or whatever method to Node which generates (edge_label, Node) tuples, or a dict, or an OrderedDict, or Edge objects if we choose to have those. Apply can implement edges with enumerate, and other node types can implement it otherwise on their own internal storage. If we want to manipulate the inputs list, we will need to make sure that we have an Apply, but we probably want to do this anyway.
All I'm asking for, basically, is an interface that lets us write an inference mechanism that's not going to break when we want to run it on an extended IR that supports more stuff. I suppose the inputs list is all right if it is Apply-specific (after all, it doesn't exactly make sense for any other node).

Agreed. I don't want to shut down the discussion, but if we agree that certain functionality can be added later, maybe we should move the discussion to a separate issue so that it is clear that this is not a blocking issue for merging this PR.

Whether it is a blocking issue or not is what we have to determine, because some of the wide picture may require adjustments to this representation. This is the most important part to get right, we can take the time. For what it's worth, I don't think we're too far from something we can both be happy about.

But -- abstractly speaking -- what kind of container is the set of outgoing edges from a graph?

If you represent a graph of nodes with ordered inputs, I think the answer is that incoming edges are a list of nodes (MutableSequence) and outgoing edges are an unordered set of nodes alongside with their indices (Set[Tuple[Node, int]]), which is exactly what they are right now.
If you consider a graph with unordered inputs, then both the inputs and outputs should be a MutableSet.
We should pick one of these two data structures. A graph with ordered inputs is per definition more expressive than one with unordered inputs. A graph that is sometimes one and sometimes the other will just complicate our algorithms (some of which will require ordered sets of edges, for example).

But there are many operations we want to do generically on all node types, like checking reachability, matching inputs to parameters, replacing some edge, and so on. The code to do this using an inputs list will fail to generalize.

I don't understand why this is true. A list of inputs is about as generic as it gets. We are trying to represent a graph after all, and graphs are per definition nodes with lists of inputs. If you want to add labels to edges in an extended IR, just add a list of labels, which can be kept in sync with the list of inputs automatically if you want:
class ExtNode:
    def __init__(*args, **kwargs):
        super().__init__(*args, **kwargs)
        labels: List[str] = []
        inputs = ExtInputs(self)

class ExtInputs:
    def __setitem__(self, index, value):
        self.node.labels[index] = value
        ...

So here's an idea: remove inputs from Node -- only the Apply node will have this field. [...]

I think there is significant value to giving all nodes a common interface. It makes the graph data structure heterogeneous, which allows graph algorithms like reachability and topological sorting to be expressed agnostic to the type of node. This makes them easier to code, and far more robust e.g. we can add new node types without any of the algorithms being affected, because all they rely on is that each node has a list of inputs they can iterate over.

A graph with ordered inputs is per definition more expressive than one with unordered inputs.

I'm talking about a graph with labeled inputs, which is more expressive than both of these.

We are trying to represent a graph after all, and graphs are per definition nodes with lists of inputs.

You mean a particular sort of labeled multidigraph is nodes with lists of inputs.
class ExtNode:
    def __init__(*args, **kwargs):
        super().__init__(*args, **kwargs)
        labels: List[str] = []
        inputs = ExtInputs(self)

class ExtInputs:
    def __setitem__(self, index, value):
        self.node.labels[index] = value
        ...

Wait, how is that supposed to work? You are setting the input at some index with a label? But value should be a Node, shouldn't it? ExtInputs ought in fact to have a label as the index, and associate that label to the node. But then what's the list? At best it seems like this would have to be some kind of perverted Lua table that offers a list interface without segregating integer indexes from string indexes.

I think there is significant value to giving all nodes a common interface. It makes the graph data structure heterogeneous, which allows graph algorithms like reachability and topological sorting to be expressed agnostic to the type of node. This makes them easier to code, and far more robust e.g. we can add new node types without any of the algorithms being affected, because all they rely on is that each node has a list of inputs they can iterate over.

I'm not arguing against a homogeneous interface, I am arguing that a MutableSequence of inputs is an interface that only makes sense for the Apply node. Think of the operations on list: __iter__, __getitem__, __setitem__, __delitem__, append, insert, and so on. Which of them make any sense for Parameter or Constant? Just __iter__. Which of them make any sense for Return? __iter__, __getitem__ and __setitem__ (as long as the item is 0). It's not that the other methods are pointless on these nodes -- it is flat out wrong to use them on these nodes. So why offer them at all? To enshrine the complete interface of a MutableSequence in the protocol of Node is to offer a wide gamut of operations that are nonsensical in the context of most node types.
So again, what I'm saying is that the common interface for all nodes shouldn't be a list, because only one node type organizes its inputs as a list (it's not even a particularly good interface for graph traversal since it might yield duplicate nodes -- it's a detail, but I did get burned by that once). What we want as a common interface would be iteration over successors and replacing edges, because these are about the only things we can do generically.
And if we can add a way to iterate over (label, Node) pairs we can implement certain things, such as running an inference step from a function's arguments to its parameters, more generically (because we can match both sets using the labels). Labels can also make collecting some information a bit easier if they have semantics, e.g. if an edge with the FN label signals that its target is called by that node, that seems a pinch better than checking whether it is the 0th input of an Apply node.

Now, returning to debugging information about edges, here's a situation where it would likely be useful. Suppose you have some call f(x, y) where y has the wrong type. Ideally you would want to show a caret right under y; however, the node for y points to its definition, and if you point to f it's not entirely clear which argument is the wrong one. The only sensible place to store that information is in the edge toward y.
It may be possible to dig it out from the original Python AST node by searching for a Name node that refers to the same name as the definition, but that's hardly robust: it'll fail if there are multiple occurrences of that name, for example.
This being said, it does seem that tracking that information would be delicate. I reckon most languages simply don't do type inference on a graph representation, and then they only bother to track calls.

I'm talking about a graph with labeled inputs, which is more expressive than both of these.

How so? The labels are just sugar. Anything that can be done with labeled inputs can be done with ordered inputs. Just give each index a label, and if you want to allow for unused labels, allow None as a input.

Wait, how is that supposed to work?

I had changed it to be __delitem__. Either way, I won't write out the full example. The idea is simple: A labeled set of inputs is simply an ordered list of inputs with an accompanying ordered list of labels. If you delete an input, make sure you delete the label, etc. Hence it is trivial to build a graph with labeled inputs on top of a graph with ordered inputs.

What we want as a common interface would be iteration over successors and replacing edges, because these are about the only things we can do generically.

This confuses me.
We need to be able to iterate over both predecessors and successors. Constant propagation, type inference, etc. are forward analyses, whereas dead code elimination requires backward analysis. Whether incoming edges, outgoing, or both, are mutable is a different question. I would inclined to keep only incoming edges mutable (as in the current code) until we come across an algorithm which is more naturally expressed by manipulating outgoing edges.
We want to be able to do more than just replacing edges. For example, if a function parameter turns out to be unused (after optimization), we might want to remove it entirely. We can then proceed by deleting all the corresponding arguments, so that we can interprocedural dead code elimination. We already discussed inserting monads, which might involve appending arguments to function calls. So, we want to deletion, insertion, and replacement. That's the MutableSequence interface right there.

I'm not arguing against a homogeneous interface [...] To enshrine the complete interface of a MutableSequence in the protocol of Node is to offer a wide gamut of operations that are nonsensical in the context of most node types.

It seems that you are arguing that all the operations that are "nonsensical" for each node should be explicitly disallowed i.e. Apply should have a fully mutable list, but Return's list should be forced to be of size 1, whereas Parameter and Constant should have a tuple of of length 0 as inputs.
This then seems to have turned into a subject discussion on how defensively we should program. Should we explicitly check each operation to make sure that what the user is trying to do is valid, and throw errors whenever they do something that might be a bug?
For example, I might want to write code like return_.inputs.pop() followed by return_.inputs.append(...), or maybe return_.inputs.append(...) followed by del return_.inputs [0]. Whether or not this is the "ideal" way of replacing a Return node's inputs is questionable of course, but a framework should enable workflows, not force them.
So I would argue that enforcing all these constraints is needlessly defensive. The downsides of defensive programming are that they increase the amount of code to write and maintain, and they can result in a framework that will throw errors even when the user knows what they are doing.
An alternative approach:
def validate(node):
    """Validate an ANF graph.

    Use this to ensure that your optimization pass resulted in a valid graph.
    """
    for n in dfs(node):
        if isinstance(n, (Constant, Parameter)) and n.inputs:
            raise ValueError
        if isinstance(n, Return) and len(n.inputs) != 1:
            raise ValueError
        if not all(isinstance(i, Node) for i in n.inputs):
            raise ValueError
        ...

Labels can also make collecting some information a bit easier if they have semantics, e.g. if an edge with the FN label signals that its target is called by that node, that seems a pinch better than checking whether it is the 0th input of an Apply node.

Labels don't need to be part of the data structure for these kind of conveniences. We could e.g. add this to Apply:
@property
def arguments(self) -> List[Node]:
    return self.inputs[1:]

@property
def function(self) -> Node:
    return self.inputs[0]

and/or this to Node:
def called_by(self) -> Iterable[Node]:
    for n, i in self.uses:
        if isinstance(n, Apply) and i == 0:
            yield n

It is better to have a simple data structure with appropriate helper functions to inspect and manipulate it, than to try to jam everything into it.

How so? The labels are just sugar. Anything that can be done with labeled inputs can be done with ordered inputs. Just give each index a label, and if you want to allow for unused labels, allow None as a input.

They are not sugar. Are we talking about the same thing? Maybe we are not. In the ordered case, I consider the indexes to be the labels -- label 0 corresponds to the function, label 1 to the first argument, and so on. That correspondence is implicitly enforced by the way the list is used, e.g. when they are zipped along function parameters. In the case of a hypothetical extended node that supports keyword arguments, then some labels may be parameter names -- strings. You cannot simply match them with integer indexes, because in a call like f(a=x, b=y) you don't necessarily know whether a is f's first or second argument. It's just a different calling protocol.
If inputs are only indexed by integers, the mapping node.inputs[k] -> graph.parameters[k], which is what you would normally do, cannot work for ExtApply and a more convoluted scheme has to be used. Code written under the assumption that it has an Apply node may not actually raise exceptions, because it would get the same data structure, but it would be wrong, because it would use the wrong matching logic. This suggests that adding this new functionality after the fact may prove difficult.

This confuses me.

The way I visualize the graph, the arrows flow from outputs to inputs, so I've been calling the inputs successors/outgoing edges, because that's basically what they are. Is that what's confusing? Sorry about that.

We want to be able to do more than just replacing edges. For example, if a function parameter turns out to be unused (after optimization), we might want to remove it entirely. We can then proceed by deleting all the corresponding arguments, so that we can interprocedural dead code elimination. We already discussed inserting monads, which might involve appending arguments to function calls. So, we want to deletion, insertion, and replacement. That's the MutableSequence interface right there.

None of these things involve fiddling with any node type other than Apply. If you remove a function parameter, you have to remove it from its calls, aka Apply nodes. Inserting monads, same thing.

This then seems to have turned into a subject discussion on how defensively we should program. Should we explicitly check each operation to make sure that what the user is trying to do is valid, and throw errors whenever they do something that might be a bug?

Explicitly check what? I'm talking about doing @property def inputs(self): return () on Constant, that's pretty simple.

For example, I might want to write code like return_.inputs.pop() followed by return_.inputs.append(...), or maybe return_.inputs.append(...) followed by del return_.inputs [0]. Whether or not this is the "ideal" way of replacing a Return node's inputs is questionable of course, but a framework should enable workflows, not force them.

But I mean, if you don't want to force a workflow, why is there a class hierarchy to begin with? In master I had a single node type with is_constant etc. functions, because I thought it could perhaps be a useful workflow to be able to switch from a kind of node to another: for example, a constant that's expensive to store could be replaced by a computation, and this could be done by making it the application of a function on simpler constants. Wasn't too sure about it, but that was the idea (and I must say it seems more plausible than that weird way of rewiring Return).
With the current design, you have to mind what the class of the node is. But if every node type is capable of the same things as every other, what does an isinstance check tell you? It tells you nothing you can rely on. You might as well scrap the hierarchy and have a single Node type that can do everything.

But if every node type is capable of the same things as every other, what does an isinstance check tell you? It tells you nothing you can rely on. You might as well scrap the hierarchy and have a single Node type that can do everything.

The main reasons I added a hierarchy is to simplify initialization and to make checks more readable e.g. isinstance(n, Constant) is clearer than not n.inputs and n.value is not PARAMETER, and Parameter(g) is nicer than Parameter([], PARAMETER, g). But yes, beyond those niceties the hierarchy could just be scrapped, just like in your original code. My intention was for their core interfaces to be entirely shared.

Explicitly check what? I'm talking about doing @property def inputs(self): return () on Constant, that's pretty simple.

It is less straightforward for Return.
And here's an example of code that would no longer work if you set inputs to be a read-only attribute on Constant. This is not a contrived example; you need this if you want to copy nodes, else uses will contain incorrect information.
class Node:
    ...
    def __copy__(self):
        cls = self.__class__
        obj = cls.__new__(cls)
        Node.__init__(obj, self.inputs, self.value, self.graph)
        return obj

class Constant(Node):
    ...

c1 = Constant(2)
c2 = copy.copy(c1)  # copies the node correctly

If you change the interface so that inputs is sometimes writable and sometimes not, those nodes would have to define their own copy constructors. I am worried about these kind of issues occurring in more places.

But I mean, if you don't want to force a workflow, why is there a class hierarchy to begin with? In master I had a single node type with is_constant etc. functions, because I thought it could perhaps be a useful workflow to be able to switch from a kind of node to another.

Yes, I think that's a reasonable reason to switch back to having a single Node class. There are advantages that a hierarchy gives you over is_constant style checks though: (a) simpler initialization, (b) static type checking with mypy, and (c) node-specific methods.
If necessary, changing a Node to a different type could still be implemented with something like this (note that this too relies on all node types having the same interface):
class Node:
    ...
    def replace(self, other):
        """Replace one node in the graph with another."""
        other.inputs = self.inputs
        for n, i in self.uses:
            n.inputs[i] = other
        self.inputs.clear()

    def cast(self, cls):
        """Change one node type into another."""
        obj = cls.__new__(cls)
        Node.__init__(obj, self.inputs, self.value, self.graph)
        self.replace(obj)
        return obj

If inputs are only indexed by integers, the mapping node.inputs[k] -> graph.parameters[k], which is what you would normally do, cannot work for ExtApply and a more convoluted scheme has to be used. Code written under the assumption that it has an Apply node may not actually raise exceptions, because it would get the same data structure, but it would be wrong, because it would use the wrong matching logic. This suggests that adding this new functionality after the fact may prove difficult.

So in summary: My understanding is that you think it would be a good idea for an extended IR to have support for labeled inputs. Moreover, you would like the core IR and extended IR to have a unified interface that allows some algorithms to operate on both. For clarity sake, let me summarize some ways of approaching this that I can think of or that have been mentioned:
Firstly, you could ensure that the Liskov substitution principle is met: Looping over the inputs should yield nodes in order. You can induce this ordering through your labels. That is, if you make sure that your lists of arguments and parameters are always ordered so that the corresponding lists of labels are sorted, any logic that relies on iterating over ordered inputs still works. This is what I meant when I said that labels are just sugar.
from bisect import bisect_left

class ExtInputs(MutableMapping):
    """Effectively a dictionary that keeps its keys sorted.

    If needed this could be extended to support integer indexing
    for `__getitem__` and `__delitem__`.

    Note that this violates the Python ABC: It iterates over values instead of keys.
    """
    def __init__(self, node: Node, init=None) -> None:
        self.inputs = Inputs(node)
        self.labels: List[str] = []
        if init is not None:
            for k, v in init.items():
                self[k] = v
    def _index(self, key: str) -> int:
        i = bisect_left(self.labels, key)
        return self.labels[i:].index(key) + i
    def __getitem__(self, key: str) -> Node:
        return self.inputs[self._index(key)]
    def __setitem__(self, key: str, value: Node) -> None:
        i = bisect_left(self.labels, key)
        self.labels.insert(i, key)
        self.inputs.insert(i, value)
    def __delitem__(self, key: str) -> None:
        i = self._index(key)
        del self.inputs[i]
        del self.labels[i]
    def __iter__(self) -> Iterable[Node]:
        for node in self.inputs:
            yield node
    def __len__(self):
        return len(self.inputs)

class ExtApply(Apply):
    def __init__(self, inputs: Dict[str, Node], graph: Graph) -> None:
        super().__init__([], None, graph)
        self.inputs = ExtInputs(self, inputs)

Now zip(ext_apply.inputs, function.parameters) works just fine (assuming that the function parameters are also sorted). However, the fact that __iter__ doesn't have the right signature is a canary in a coal mine. This container now tries to be a dictionary as well as a list, which can get awkward.
So the second option is to hide certain operations behind helper functions e.g. match_parameters can check whether it is dealing with an Apply or ExtApply node and match the parameters appropriately. However, this clearly breaks the encapsulation principle (match_parameters needs to be aware of the implementation and details of all versions of Apply).
The third option is what I think you proposed earlier: Have separate inputs implementations but have helper methods e.g. Apply has a list for inputs, ExtApply has a dict, but both have an edges method that will always return an iterable over predecessor/successor nodes. This decouples the implementation, but it violates the substitutability principle e.g. zip works but gives the wrong result, as you mentioned, unless the user knows that he must use the special edges method.
A third option is to design a custom "inputs" container. This would basically be the list/dict hybrid from my code snippet e.g. iterating over inputs should always produce the nodes instead of the labels, indexing should be possible using integers as well as strings, etc. The downside of this approach is that we cannot use Python's built-in abstractions. Instead, people will be dealing with this custom interface which is trying to sometimes be a list, sometimes a dict, but never fully either. More importantly, complexity from the extended IR is leaking into the core IR, which means we are increasing coupling and reducing cohesion.
The fourth option is to just throw substitutability out the window. Allowing the extended IR to be incompatible with the core IR means that it can support a lot more complexity (default arguments, varargs, etc.), without this complexity leaking into the core IR. The downside is that you can't expect any algorithm that works on the core IR to automatically work on the extended IR and vice versa.
The fifth option is to just throw lists out of the window, and have an IR that has dictionaries of inputs only (i.e. unordered but labeled). Issues would be that each input must be given a unique name, ordering goes out the window, and we end up using hash tables everywhere.
In the end, I think that your suggestion of having a separate edges method (option 3) is the right approach, but the extended IR shouldn't inherit from the core IR (option 4).
class Node(metaclass=ABCMeta):
    @property
    @abstractmethod
    def edges(self) -> Iterable['Node']:
        pass

class ANFNode(Node):
    def __init__(self, inputs: List['ANFNode']) -> None:
        self.inputs = inputs

    def edges(self) -> Iterable['ANFNode']:
        return self.inputs

class ExtNode(Node):
    def __init__(self, inputs: Dict[str, 'ExtNode']) -> None:
        self.inputs = inputs

    def edges(self) -> Iterable['ExtNode']:
        return self.inputs.values()

def dfs(root: Node):
    seen = set()
    to_visit = [root]
    while to_visit:
        node = to_visit.pop()
        seen.add(node)
        yield node
        for n in node.edges:
            if n not in seen:
                to_visit.append(n)

The advantage is that the graphs remain clean and simple: They are either lists or dictionaries, not some strange mix. This means the user can manipulate inputs as they please on each representation, including calls to update, insert, etc. Some algorithms will be written for one of the two IRs only (e.g. algorithms that rely on matching arguments to parameters with support for default arguments), but others can be made to support both as long as they rely on the abstractions defined in the base class.
Of course, with Python duck typing you can still screw this up, but that's where mypy would come in...

The main reasons I added a hierarchy is to simplify initialization and to make checks more readable e.g. isinstance(n, Constant) is clearer than not n.inputs and n.value is not PARAMETER, and Parameter(g) is nicer than Parameter([], PARAMETER, g). But yes, beyond those niceties the hierarchy could just be scrapped, just like in your original code. My intention was for their core interfaces to be entirely shared.

Niceties are easy enough to add without classes, e.g. n.is_constant() to check if a node is constant, or Node.make_constant(v) to create a constant node. It's less symbols to imports, too.

If you change the interface so that inputs is sometimes writable and sometimes not, those nodes would have to define their own copy constructors.

Hadn't really thought of that, but that doesn't really bother me.

Firstly, you could ensure that the Liskov substitution principle is met: Looping over the inputs should yield nodes in order. You can induce this ordering through your labels. That is, if you make sure that your lists of arguments and parameters are always ordered so that the corresponding lists of labels are sorted, any logic that relies on iterating over ordered inputs still works. This is what I meant when I said that labels are just sugar.

That won't work, because some parameters can be given by index or by name. So if you have f(x, y) and g(y, x), imposing a label ordering (let's say alphabetical) would swap g's arguments. Which means that calling g(1, 2) would match x with 1 and y with 2. Optional arguments also defeat this scheme.

In the end, I think that your suggestion of having a separate edges method (option 3) is the right approach, but the extended IR shouldn't inherit from the core IR (option 4).

Yeah, that will do. It's only a subset of the algorithms we may want to run on both, and I don't think that subset involves mutation anyway.
That might also solve the debugging-info-in-edges debate: the parser's IR could include that information, which as I mentioned earlier would help give better messages for type errors, but it could be thrown away later if we don't want to deal with all that in the optimizer.

That won't work, because some parameters can be given by index or by name.

This solution was based on the assumption that arguments are always given by name. Doing both names and indices requires something more complicated, obviously.

Hadn't really thought of that, but that doesn't really bother me.

It does bother me a bit. This kind of stuff often plays a role with e.g. deserialization when you need to reconstruct objects.

Niceties are easy enough to add without classes

Sure. I personally still have a small preference for the class hierarchy, but could be persuaded otherwise. Main advantages I see:

It is clear that Node normally shouldn't be instantiated if you consider it an ABC with a set of concrete children (even though Python doesn't enforce this), but it is less obvious when you just have a series of constructor class methods.
isinstance(n, Return) is more standard than n.is_return()
A hierarchy clarifies some things e.g. isinstance(n, Return) and isinstance(n, Constant) can never be true if Return and Constant are siblings, but that is not obvious for n.is_return() and n.is_constant()
Prints better (could be fixed by custom __repr__ method, but you get it for free this way)
Ability to add node-specific properties and methods e.g. Apply.function and Apply.arguments
More structured code (separate classes with each their own docstrings instead of a monster class with many constructor methods)

I added the Node interface which only supports iterating over incoming and outgoing edges. I put this in ir.py and move the core IR into anf_ir.py and renamed Node to ANFNode appropriately. I'm also open to just calling it core_ir.py and renaming it to CoreNode, or something else.
I added a DFS function as an example of using this interface, along with a test.
I also added the __copy__ constructor (since I had written it anyway)
I changed the debug attribute to a custom object which inherits from SimpleNamespace. This basically behaves the same, but gives us a place to specify types and documentation for standard debug attributes.

Current code I think resolved most of the things we discussed, and has 100% coverage. Good to merge?

A hierarchy clarifies some things e.g. isinstance(n, Return) and isinstance(n, Constant) can never be true if Return and Constant are siblings, but that is not obvious for n.is_return() and n.is_constant()

Arguably, Return is the one node that might warrant a different class, since it's the one node type that has no functional purpose, cannot be replaced, cannot die, cannot have a value, always has an empty uses set, and has different semantics for inputs (inputs[0] is not a function). Its only purpose is to sit there, anchor the graph, and figure in another node's uses to simplify replacement operations. I'd probably make it a sibling of ANFNode, to be honest.
As for the other nodes, it is conceivable that e.g. is_constant() and is_apply() both return true, if one decides that a constant should be recomputed each time instead of stored, but nonetheless leaves its value in the graph so that constant propagation can work. That's precisely the kind of flexibility gained by making them the same node type.

Ability to add node-specific properties and methods e.g. Apply.function and Apply.arguments

That can still be done on ANFNode, if we assume that input 0 always corresponds to a function, and inputs 1: always correspond to arguments. In that case, Return would need to store the root node in inputs[1] instead of inputs[0], and presumably None or some special token (a return primitive?) in inputs[0]. That makes the semantics more consistent, so it's not a terrible idea (but using a different node type for Return might be better).

Current code I think resolved most of the things we discussed, and has 100% coverage. Good to merge?

Let me just read everything over and make a last review.

In that case, Return would need to store the root node in inputs[1] instead of inputs[0], and presumably None or some special token (a return primitive?) in inputs[0]. That makes the semantics more consistent, so it's not a terrible idea [...]

I'd not be opposed to getting rid of Return entirely and just replacing it with an Apply node that calls the primitive Return, which has the signature Any ->. As you mentioned, the semantics would be more consistent and we simplify the representation a bit. For example, type inference might be easier if we can just treat Return as a normal function, since now inferring its argument types is equivalent to inferring the return type of the function.
The reason we introduced Return nodes after all was for cases such as this example you gave:
def f(x):
    a = x * 1
    def g():
        return a
    return g

Since a.graph == f, it's not obvious from the graph alone that a is closed over (since that information would have to be deduced from the fact that g.return_ == a). That is, you can't do stuff like used_in_functions = set(n.graph for n in self.outgoing). But all of that is also solved as well by having g.return_ = Apply([RETURN_OP, a], g).

Agreed. Let's replace Return(a, g) by Apply([RETURN_OP, a], g).

I'm done with reviewing this, so unless anyone else has comments, you can merge it.

Woohoo! 🎉


Fixed issue with contentUrl for attachments.

Fixes #1362
===
Also moved the service url declaration out of the for loop so that it's not unnecessarily generated for every item.

Pull Request Test Coverage Report for Build 2181

1 of 1 (100.0%)  changed or added relevant line in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 54.962%

Totals

Change from base Build 2173:
0.0%

Covered Lines:
4871

Relevant Lines:
8322

💛  - Coveralls


Add permission to codeunit 11402 'Post Code Lookup - Table'

Hello,
Can you please add read permission on Record "Post Code Range" in codeunit 11402 "Post Code Lookup - Table"?
Desired solution:
Permissions = tabledata "Post Code Range" = r;
Thank you.

Thanks for reporting this. We agree, and we’ll publish a fix asap, either in an update for the current version or in the next major release. Please do not reply to this, as we do not monitor closed issues. If you have follow-up questions or requests, please create a new issue where you reference this one.
Build ID: 47752.


Handle be error after purchase

Task/Issue URL: https://app.asana.com/0/488551667048375/1206350282230199/f
Description
See task
Steps to test this PR
See task

[!WARNING]
This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

#4304  👈
#4297
#4291
#4233
develop

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @marcosholgado and the rest of your teammates on  Graphite


QoL : Customizable vault button icon

Persisted vault tab location
This QoL improvement allow TQVaultAE to remember last selected tab between vault switching and tool restart.
it remember selection individualy for left and right vault.

"Disabled Tooltip" feature become persistent
This improvement allow TQVaultAE to remember tab tooltip enabling/disabling between restart.

New vault file format
This update also releive TQVaultAE from the burden of the old and clunky binary format for Vault storage and change it to Json.
The conversion from .vault to .json is made the first time the vault is loaded then the Json file become the prefered file format.
This doesn't affect the other files that have to be shared with the game.
Newly created vaults use json from the start.

Customizable vault button icon
Open the wizard.

Customize the button

Result (with label)

Updated ArzExplorer

Added Textbox displaying the file path inside the archive (double click copy the value into the clipboard).
Added Statusbar displaying detailed informations about current selection.

Alright this is fantastic. Unfortunately it conflicts, but I'm looking forward to putting that in.

Alright this is fantastic. Unfortunately it conflicts, but I'm looking forward to putting that in.

@EtienneLamoureux  Thanks it was not an easy task :)
It's merged.


Allow SO_REUSEADDR on Windows UDP sockets.

This pull request allows SO_REUSEADDR to be used on Windows UDP sockets. This keeps the TCP protection in place; but allows multiple UDPServer instances to receive the same packets. This should make Windows and Linux behave identical with regards to UDPServer instances.
The following video demonstrates a MoCap application sending UDP skeleton packets to both a Godot application, and the Godot editor tool-script.
https://github.com/godotengine/godot/assets/1863707/7a0b6b92-65f3-41c1-b7df-054a9d384c1b

This pull request allows SO_REUSEADDR to be used on Windows UDP sockets. This keeps the TCP protection in place; but allows multiple UDPServer instances to receive the same packets. This should make Windows and Linux behave identical with regards to UDPServer instances.

SO_REUSEADDR doesn't really work that way on *nix system.
It does not allow routing of a regular single packet to multiple bound sockets.
You can test this with the following script:
extends Control

var udp_server1 := UDPServer.new()
var udp_server2 := UDPServer.new()

var udp_send := PacketPeerUDP.new()

# Called when the node enters the scene tree for the first time.
func _ready():
	print(udp_server1.listen(4343))
	print(udp_server2.listen(4343))

	udp_send.connect_to_host("<IP_ADDRESS>", 4343)
	udp_send.put_packet(PackedByteArray([1,2,3,4]))

	udp_server1.poll()
	udp_server2.poll()
	print("Server 1 ", udp_server1.is_connection_available())
	print("Server 2 ", udp_server2.is_connection_available())

Will print:
0
0
Server 1 false
Server 2 true

As you can see, the packet was routed to the last bound socket, not all of them.
As far as I understand this is also the behavior on Windows without SO_REUSEADDR which instead has a completely different meaning on Windows.

My prior experience with this has been combining SO_REUSEADDR with broadcast packets (network discovery) rather than unicast, and in those cases all applications bound using SO_REUSEADDR receive the same broadcast UDP packet.

Indeed, as far as I know that is the behavior on Linux for broadcast and multicast packets, but not for unicast packets

All right, this PR does NOT do what I had intended, but still may be useful - indeed I had incorrectly applied my experience with broadcast packets inappropriately to unicast packets, and numerous documentation/discussions make it clear that the sockets API cannot be used to implement multiple applications receiving unicast packets on the same computer.
Instead this change allows Windows to support multiple UDPServers listening to the same broadcast packets just as is supported on Linux today.
Consider the following minor modification to the previous code from @Faless:
extends Control

var udp_server1 := UDPServer.new()
var udp_server2 := UDPServer.new()

var udp_send := PacketPeerUDP.new()

# Called when the node enters the scene tree for the first time.
func _ready():
	print("Operating System: ", OS.get_distribution_name())
	print("Server 1 listen ", udp_server1.listen(4343))
	print("Server 2 listen ", udp_server2.listen(4343))

	udp_send.set_broadcast_enabled(true)
	udp_send.set_dest_address("<IP_ADDRESS>", 4343)
	udp_send.put_packet(PackedByteArray([1,2,3,4]))

	udp_server1.poll()
	udp_server2.poll()
	print("Server 1 ", udp_server1.is_connection_available())
	print("Server 2 ", udp_server2.is_connection_available())

With Godot 4.2.1 on Linux we get both UDPServer instances receiving the same broadcast packet:
Operating System: Ubuntu 23.10
Server 1 listen 0
Server 2 listen 0
Server 1 true
Server 2 true

With Godot 4.2.1 on Windows we get a failure to start listening on the second UDPServer because the port is busy:
Operating System: Windows
Server 1 listen 0
Server 2 listen 2
Server 1 true
Server 2 false

With this patch on Windows we now get both UDPServer starting correctly and receiving the same broadcast packet:
Operating System: Windows
Server 1 listen 0
Server 2 listen 0
Server 1 true
Server 2 true

It doesn't solve the original problem I was trying to solve; however it does appear to meet the updated PR description (thanks @akien-mga) and normalizes some network behavior bringing Windows behavior in line with Linux for broadcast packets.


Make TableSearch flexible for user defined filter

let the user bind the search box inputValue for their own filter logic
Ref: https://github.com/themesberg/flowbite-svelte/issues/117

Closes # 117
📑 Description
let the user bind the search box inputValue for their own filter logic
syntax

[ ] Not Completed
[x] Completed

✅ Checks

[x] My pull request adheres to the code style of this project
[ ] My code requires changes to the documentation
[ ] I have updated the documentation as required
[ ] All the tests have passed
[x] My pull request is based on the latest commit/version

If your branch is behind the upstream master, I will have difficulties to merge the request. Please fetch the latest version of the branch:
git checkout your-branch
git pull upstream main

or
git checkout your-branch
git pull --rebase upstream main

ℹ Additional Information

CRUD table example:

{#if error}

{error.code}

{error.details}

{/if}

<TableSearch placeholder="Search by subject name" hoverable={true} bind:inputValue={searchTerm} >
	<TableHead>
		<TableHeadCell>Name</TableHeadCell>
		<TableHeadCell>Subject ID</TableHeadCell>
		<TableHeadCell>Source</TableHeadCell>
		<TableHeadCell>Destination</TableHeadCell>
		<TableHeadCell>Enforcement</TableHeadCell>
		<TableHeadCell>Delete</TableHeadCell>

	</TableHead>
	<TableBody class="divide-y">
		{#each filteredItems as item}
			<TableBodyRow>
				<TableBodyCell>
					{item.displayName?.value}
				</TableBodyCell>
				<TableBodyCell>{item.subject?.id?.value}</TableBodyCell>
				<TableBodyCell>{item.rule?.sourceAddress?.value}</TableBodyCell>
				<TableBodyCell>{item.rule?.destinationAddress?.value}</TableBodyCell>
				<TableBodyCell>{item.enforcement ?? 0}</TableBodyCell>
				<TableBodyCell>
					<button on:click|preventDefault={deletePolicy(item.id?.value)}> <Archive class="text-pink-700 dark:text-blue-300"/> </button>
				</TableBodyCell>
			</TableBodyRow>
		{/each}
	</TableBody>
</TableSearch>

I need sometime for this. I will get back to you asap. Please be patient with me.
Thank you.


Add a notification when file download is complete

When I downloaded /bin/sh, I couldn't tell when it finished

This already happens, I didn't notice it before


API connection

Attention Please
In this Project, I have used

CSS
JavaScript
React
ES6 Modules
API
following linters standards.
following Github flows standards.
following git flows standards.
webpak

Use the command  to start the server
npm start

In PR #7

[x] - I have added an Action Creator, that returns a function.
[x] - I have Fetched a list of books.
[x] - I have dispatched another action (add an action type) that will pass the API response to the reducer and update the state.
[x] - I have changed my existing Action Creators, so they return functions.
[x] - I have made changes by using API.
[x] - I have dispatched an action (action type that existed before refactor) that will update the state accordingly.

Have fun 😊😉

Hi @zakiyousufi,
Great work on making the changes requested by a previous reviewer clap You've done well implementing some of the requested changes, but there are still some which aren't addressed yet.
Highlights

[ ]  Nice job fixing the browser console errors which was preventing the app from running rocket

Required Changes
Check the comments under the review.
Optional suggestions
Every comment with the [OPTIONAL] prefix won't stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better. Some of them were simply missed by the previous reviewer and addressing them will really improve your application.
Cheers and Happy coding!clapclapclap
Feel free to leave any questions or comments in the PR thread if something is not 100% clear. Please, remember to tag me in your question so I can receive the notification.
Please, do not open a new Pull Request for re-reviews. You should use the same Pull Request submitted for the first review, either valid or invalid unless it is requested otherwise.
As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.

here is my comment

Hi @zakiyousufi,
Your project is complete! There is nothing else to say other than... it's time to merge it :shipit: Congratulations! tada
Highlights

[ ]  Nice job fixing the browser console errors which was preventing the app from running rocket

Cheers and Happy coding!clapclapclap
Feel free to leave any questions or comments in the PR thread if something is not 100% clear. Please, remember to tag me in your question so I can receive the notification.
As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.

here we go


[stable/heapster] service name causes top to fail

When using the stable/heapster chart I'm seeing kubectl top nodes fail with:
Error from server (NotFound): the server could not find the requested resource (get services http:heapster:)

I was wondering if heapster might need additional labels such as
kubernetes.io/cluster-service: "true"

however it looks like the service needs:

running in the kube-system namespace
named heapster

Running in the correct namespace could be left to the user using the --namespace flag, but should we consider dropping https://github.com/kubernetes/charts/blob/master/stable/heapster/templates/service.yaml#L4 to .Chart.Name?

cc @jose5918

@mikesplain Do you think something along these lines would work for you?https://github.com/jose5918/charts/commit/109b6aa2644b06f4754fd9826845a1e35f809d29

@jose5918 Yep! That's perfect, though I think you have a typo here:
https://github.com/jose5918/charts/blob/109b6aa2644b06f4754fd9826845a1e35f809d29/stable/heapster/templates/service.yaml#L10
I had to change it to .Values.service.labels

@mikesplain Thanks for catching that. Didn't have a chance to test it yesterday

@jose5918 Any plans to send out a PR for this?
I think the way heapster chart is currently set up is surprising (e.g. I expected our horizontal pod autoscalers to work out of the box after installing the chart)

@ferminromero Sorry about that. Thank you for reminding me. I had forgotten a few things since I was busy moving recently.


Install Error: Plugin Java method Reference Diagramm depends on unknown plugin com.intellij.diagram

Hello
Thank you for sharing your great work!
Unfortunately, Release 3.1.1 can not be installed, during installation one gets:

After restarting IntelliJ, it reports:

Could it be that the plugin is not compatible with IntelliJ Community Edition?
Thanks a lot, kind regards,
Thomas

Yes, this plugin is not compatible with community edition.

The reason is, that I used the UML capability of IDEA which is a licensed library and thus jetbrains just has it in pro version.
For me it would be better to have the plugin be compatible with community edition, but then I have to implement the graph and the integration with Goto Source etc. for my self. For that, I do not have the time.

Hello @Stefku
thanks a lot for your help!,
I had unfortunately overlooked that you explicitly mentioned 'This plugin for IntelliJ IDEA Ultimate...' in the readme.md. I actually always read information like readme.md by skimming it and I focus on the headings.
Would it be possible for you to mention it a bit more clearly in these two places, so that community edition users don't even try to test it? (Please see then screenshot)
(1) The About text does just talk about IntelliJ
(2) if possible, it would probably be good if Ultimate is written in bold in the introductory sentence: "This plugin for IntelliJ IDEA Ultimate helps to get an overview "

Thanks a lot, kind regards,
Thomas

Thanks @schittli for the hint. I updated the recommended texts.


Related Link to SharePoint Pnp is broken

Hi, just a quick one the link to SharePoint Developer Paterns and Practices is malformed and links to:
https://aka.ms/sppnp)[Encoding](https://msdn.microsoft.com/en-us/library/system.text.encoding_properties.aspx

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: 451dc826-b072-5bcd-e98d-319ee82d73fe
Version Independent ID: c0c6ad38-e86c-cd0c-ac01-033791404e25
Content: Get-PnPProvisioningTemplate (sharepoint-pnp)
Content Source: sharepoint/sharepoint-ps/sharepoint-pnp/Get-PnPProvisioningTemplate.md
GitHub Login: @techwriter40
Microsoft Alias: kirks

@officedocsbot assign @yogkumgit

Hello @mikewaretena Miguel how are you?  👍
PnP documentation articles are directly reflected from source code.
I will need to kindly ask you to visit this other repo to open PnP issues/questions. PnP cmdlets are maintained by the community, a member of it will pick it up there. Thank you!

@get-itips  Thank you very much for the contribution and sharing this explanation. @mikewaretena
Hope this comment is helpful for you. If you see a documentation update is required, please feel free to open an issue for the same. We proceed here to close it. Thanks for taking out some time to open the issue. Appreciate and encourage you to do the same in future also.


fix(scripts): Exit validation scripts early for non-existent builds.

:tada: This PR is included in version 1.0.2 :tada:
The release is available on:

npm package (@latest dist-tag)
GitHub release

Your semantic-release bot :package::rocket:


DirectController

Hi Damien,
I've been looking at the AD B2C BFF Blazor template, it looks good, thx for great work.
I did wonder since include global anti forgery setting in Startup, why did you put [ValidateAntiForgeryToken] on DirectApiController or is this just a left over?
Thx...Bob Baldwin
aka VSDotNetGuy!

Hi Bob
No I did this extra because I have not forced this globally. All API calls need this as a fallback for same site. Of course maybe this could be improved :)
Greetings Damien


Function 'function' field is dict instead of str

Since d1f49379651d80613e2b2181a0b6e91171a016a2, the 'function' field of Function is a dict (changed from str) expected to contain args and their values. ex: https://github.com/ModECI/MDF/blob/48e2b2e22188cb83bbe36e66f9692f411ae70f02/examples/MDF/translation/Translated_ABCD.json#L122-L130
Why use this dictionary format instead of handling the function arguments in the 'args' field as with evaluable expressions? This change seems to add extra complexity

hey @kmantel, I would like to work on this issue. Could you please assign this task to me .

@kmantel I would like to work on this issue, would you please assign this issue to me.

Hi, will check back and assign one of you soon

This issue is more to track a proposed design change to the MDF formal specification for parameters and functions rather than a bug fix I'm afraid. It requires examination of the merits of the change by the MDF developers rather than something that just needs a quick change in the code, so is probably something more for the actual internship rather than the contribution phase.


fix(modal): Call hideModal in ngOnDestroy if modal is shown

Resolves #853

Current coverage is 90.40% (diff: 100%)

Merging #1038 into development will not change coverage

@@           development      #1038   diff @@
=============================================
  Files               26         26
  Lines              782        782
  Methods            142        142
  Messages             0          0
  Branches           115        115
=============================================
  Hits               707        707
  Misses              75         75
  Partials             0          0

Powered by Codecov. Last update 887c6b2...efd20f6