Hi Becket,

Thank you for the detailed reply!

My understanding of your comments is that most of option-1 looks good
except its change of the Transformer semantics. Please see my reply inline.


On Tue, Jul 20, 2021 at 11:43 AM Becket Qin <becket....@gmail.com> wrote:

> Hi Dong, Zhipeng and Fan,
>
> Thanks for the detailed proposals. It is quite a lot of reading! Given that
> we are introducing a lot of stuff here, I find that it might be easier for
> people to discuss if we can list the fundamental differences first. From
> what I understand, the very fundamental difference between the two
> proposals is following:
>
> * In order to support graph structure, do we extend Transformer/Estimator,
> or do we introduce a new set of API? *
>
> Proposal 1 tries to keep one set of API, which is based on
> Transformer/Estimator/Pipeline. More specifically, it does the following:
>     - Make Transformer and Estimator multi-input and multi-output (MIMO).
>     - Introduce a Graph/GraphModel as counter parts of
> Pipeline/PipelineModel.
>
> Proposal 2 leaves the existing Transformer/Estimator/Pipeline as is.
> Instead, it introduces AlgoOperators to support the graph structure. The
> AlgoOperators are general-purpose graph nodes supporting MIMO. They are
> independent of Pipeline / Transformer / Estimator.
>
>
> My two cents:
>
> I think it is a big advantage to have a single set of API rather than two
> independent sets of API, if possible. But I would suggest we change the
> current proposal 1 a little bit, by learning from proposal 2.
>
> What I like about proposal 1:
> 1. A single set of API, symmetric in Graph/GraphModel and
> Pipeline/PipelineModel.
> 2. Keeping most of the benefits from Transformer/Estimator, including the
> fit-then-transform relation and save/load capability.
>
> However, proposal 1 also introduced some changes that I am not sure about:
>
> 1. The most controversial part of proposal 1 is whether we should extend
> the Transformer/Estimator/Pipeline? In fact, different projects have
> slightly different designs for Transformer/Estimator/Pipeline. So I think
> it is OK to extend it. However, there are some commonly recognized core
> semantics that we ideally want to keep. To me these core semantics are:
>   1. Transformer is a Data -> Data conversion, Estimator deals with Data ->
> Model conversion.
>   2. Estimator.fit() gives a Transformer, and users can just call
> Transformer.transform() to perform inference.
> To me, as long as these core semantics are kept, extension to the API seems
> acceptable.
>
> Proposal 1 extends the semantic of Transformer from Data -> Data conversion
> to generic Table -> Table conversion, and claims it is equivalent to
>

I am a bit confused by this description of the semantic change. By "from
Data -> Data conversion to generic Table -> Table", do you mean "Table !=
Data"?

I am hoping we can clearly define the semantics of Transformer and agree on
its semantics definition within the community. Then we can see how option-1
changes its semantics and update option-1 as appropriate.

"AlgoOperator" in proposal 2 as a general-purpose graph node. It does
> change the first semantic. That said, this might just be a naming problem,
> though. One possible solution to this problem is having a new subclass of
> Stage without strong conventional semantics, e.g. "AlgoOp" if we borrow the
> name from proposal 2, and let Transformer extend it. Just like a
>

Assuming that option-1 does change the meaning of Transformer (this point
depends on the answer to the previous question), I am happy to update
option-1 to address the concern.

Here is how I would update option-1:
- Add an interface named "MLOperator". This interface will be the same as
the existing Transformer interface in option-1 except its name.
- Make Transformer a subclass of MLOperator. Because Transformer has
exactly the same API as MLOperator, no extra method is defined in
Transformer.
- Every other API/class in option-1 which deals with Transformer will now
deal with MLOperator.

This change proposed above basically renamed Transformer to address the
semantic/naming concern. Does this sound good?


> PipelineModel is a more specific Transformer, a Transformer would be a more
> specific "AlgoOp". If we do that, the processing logic that people don't
> feel comfortable to be a Transformer can just be put into an "AlgoOp" and
> thus can still be added to a Pipeline / Graph. This borrows the advantage
> of proposal 2. In another word, this essentially makes the "AlgoOp"
> equivalent of "AlgoOperator" in proposal 2, but allows it to be added to
> the Graph and Pipeline if people want to.
>
> This also gives my thoughts regarding the concern that making the
> Transformer/Estimator to MIMO would break the convention of single input
> single output (SISO) Transformer/Estimator. Since this does not change the
> core semantic of Transformer/Estimator, it sounds an intuitive extension to
> me.
>
> 2. Another semantic related case brought up was heterogeneous topologies in
> training and inference. In that case, the input of an Estimator would be
> different from the input of the transformer returned by Estimator.fit().
> The example to this case is Word2Vec, where the input of the Estimator
> would be an article while the input to the Transformer is a single word.
> The well recognized ML Pipeline doesn't seem to support this case, because
> it assumes the input of the Estimator and corresponding Transformer are the
> same.
>
> Both proposal 1 and proposal 2 leaves this case unsupported in the
> Pipeline. To support this case,
>    - Proposal 1 adds support to such cases in the Graph/GraphModel by
> introducing "EstimatorInput" and "TransformerInput". The downside is that
> it complicates the API.
>    - Proposal 2 leaves this to users to construct two different DAG for
> training and inference respectively. This means users would have to
> construct the DAG twice even if most parts of the DAG are the same in
> training and inference.
>
> My gut feeling is that this is not a critical difference because such
> heterogeneous topology is sort of a corner case. Most users do not need to
> worry about this. For those who do need this, either proposal 1 and
> proposal 2 seems acceptable to me. That said, it looks that with proposal
> 1, users can still choose to write the program twice without using the
> Graph API, just like what they do in proposal 2. So technically speaking,
> proposal 1 is more flexible and allows users to choose either flavor. On
> the other hand, one could argue that proposal 1 may confuse users with
> these two flavors. Although personally I feel it is clear to me, I am open
> to other ideas.
>
> 3. Lastly, there was a concern about proposal 1 is that some Estimators can
> no longer be added to the Pipeline while the original Pipeline accepts any
> Estimator.
>
> It seems that users have to always make sure the input schema required by
> the Estimator matches the input table. So even for the existing Pipeline,
> people cannot naively add any Estimator into a pipeline. Admittedly,
> proposal 1 added some more requirements, namely 1) the number of inputs
> needs to match the number of outputs of the previous stage, and 2) the
> Estimator does not generate a transformer with different required input
> schema (the heterogeneous case mentioned above). However, given that these
> mismatches will result in exceptions at compile time, just like users put
> an Estimator with mismatched input schema, personally I find it does not
> change the user experience much.
>
>
> So to summarize my thoughts on this fundamental difference.
>     - In general, I personally prefer having one set of API.
>     - The current proposal 1 may need some improvements in some cases, by
> borrowing something from proposal 2.
>
>
>
> A few other differences that I consider as non-fundamental:
>
> * Do we need a top level encapsulation API for an Algorithm? *
>
> Proposal 1 has a concept of Graph which encapsulates the entire algorithm
> to provide a unified API following the same semantic of
> Estimator/Transformer. Users can choose not to package everything into a
> Graph, but just write their own program and wrap it as an ordinary
> function.
>
> Proposal 2 does not have the top level API such as Graph. Instead, users
> can choose to write an arbitrary function if they want to.
>
> From what I understand, in proposal 1, users may still choose to ignore
> Graph API and simply construct a DAG by themselves by calling transform()
> and fit(), or calling AlgoOp.process() if we add "AlgoOp" to proposal 1 as
> I suggested earlier. So Graph is just an additional way to construct a
> graph - people can use Graph in a similar way as they do to the
> Pipeline/Pipeline model. In another word, there is no conflict between
> proposal 1 and proposal 2.
>
>
> * The ways to describe a Graph? *
>
> Proposal 1 gives two ways to construct a DAG.
> 1. the raw API using Estimator/Transformer(potentially "AlgoOp" as well).
> 2. using the GraphBuilder API.
>
> Proposal 2 only gives the raw API of AlgoOpertor. It assumes there is a
> main output and some other side outputs, so it can call
> algoOp1.linkFrom(algoOp2) without specifying the index of the output, at
> the cost of wrapping all the Tables into an AlgoOperator.
>
> The usability argument was mostly around the raw APIs. I don't think the
> two APIs differ too much from each other. With the same assumption,
> proposal 1 and proposal 2 can probably achieve very similar levels of
> usability when describing a Graph, if not exactly the same.
>
>
> There are some more other differences/arguments mentioned between the two
> proposals. However, I don't think they are fundamental. And just like the
> cases mentioned above, the two proposals can easily learn from each other.
>
> Thanks,
>
> Jiangjie (Becket) Qin
>
> On Thu, Jul 1, 2021 at 7:29 PM Dong Lin <lindon...@gmail.com> wrote:
>
> > Hi all,
> >
> > Zhipeng, Fan (cc'ed) and I are opening this thread to discuss two
> different
> > designs to extend Flink ML API to support more use-cases, e.g.
> expressing a
> > DAG of preprocessing and training logics. These two designs have been
> > documented in FLIP-173
> > <
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=184615783
> > >
> > 。
> >
> > We have different opinions on the usability and the ease-of-understanding
> > of the proposed APIs. It will be really useful to have comments of those
> > designs from the open source community and to learn your preferences.
> >
> > To facilitate the discussion, we have summarized our design principles
> and
> > opinions in this Google doc
> > <
> >
> https://docs.google.com/document/d/1L3aI9LjkcUPoM52liEY6uFktMnFMNFQ6kXAjnz_11do
> > >.
> > Code snippets for a few example use-cases are also provided in this doc
> to
> > demonstrate the difference between these two solutions.
> >
> > This Flink ML API is super important to the future of Flink ML library.
> > Please feel free to reply to this email thread or comment in the Google
> doc
> > directly.
> >
> > Thank you!
> > Dong, Zhipeng, Fan
> >
>

Reply via email to