sqlflow icon indicating copy to clipboard operation
sqlflow copied to clipboard

[Discussion] two ways of implementing Python API and connect to Go parser

Open typhoonzero opened this issue 4 years ago • 6 comments

proposal one

Python API:

import sqlflow
sqlflow.train("SELECT ...", feature_columns=[...], attrs={...}, engine="pai")
sqlflow.predict(...)
sqlflow.explain(...)
sqlflow.evaluate(...)

Go code using code generator to generate python code:

train_template=`import sqlflow
feature_columns = []
{% for ... %}
feature_columns.append({{ feature_column | to_python_code }})
{% endfor %}
sqlflow.train("""{{.Select}}""", feature_columns=feature_columns, ...)
`
...

proposal two

User API keeps the same:

import sqlflow
sqlflow.train("SELECT ...", feature_columns=[...], attrs={...}, engine="pai")

Yet we have a low level API, which will be called in sqlflow.train():

import sqlflow
ir = # protobuf message or JSON struct
sqlflow.inner.train(ir, engine="pai")

Go generate code to call the low level API:

irProto := proto.Serialize(ir)
template := `import sqlflow
ir = proto.Unserialize(stdin)
sqlflow.inner.train(ir, engine="pai")
`

typhoonzero avatar May 27 '20 02:05 typhoonzero

Proposal 3

In #2335 , the proposal is:

  1. The python package sqlflow provides an executable module sqlflow.execute, a simplified version is:
def execute(program:sqlflow_ir_pb2.Program) -> str:
    """
    `execute` executes the SQLFlow `program` as an Argo workflow and returns the job id
    """
    pass
  
def main(mode:str):
    """
    `main` parses a `sqlflow_ir_pb2.Program` from stdin and call `execute`
    """
    pass
  
if __name__ == '__main__':
    main()
  1. Go simply spawns a process rather than generates any python code:
cmd := exec.Command("python", "-m", "sqlflow.execute")
cmd.Stdin = proto.Serialize(ir)
cmd.Run()
  1. Any API can be derived from the function sqlflow.execute.execute if neccesary, for example:
# sqlflow.train fills `sqlflow_ir_pb2.Program` from arguments and calls `sqlflow.execute.execute`
sqlflow.train("SELECT ...", feature_columns=[...], attrs={...}, engine="pai")

Why Proposal 3 is superior

  1. Proposal 1 is a typical non-layered architecture. There is no difference in essence between proposal 1 and the current architecture, because it doesn't fundamentally solve the problems of the current architecture illustrated in #2335:

    • There'll be many boilerplate Go functions and Go templates that tightly couples with Python functions
    • ML Code will still scatter around Python, Go, and Go template, any modification to Python code will affect their Go and Go template counterpart
  2. Proposal 3 is a layered architecture, which draws a clear line between Python and Go. It solved the problem of the current architecture as explained in #2335 .

    • Go code has only a minimal dependency on Python code: it depends on a Python module rather than the implementation details of the Python module.
  3. Proposal 2 is a compromise of Proposal 3 and Proposal 1, in fact, it has the same defect as Proposal 1

shendiaomo avatar May 27 '20 12:05 shendiaomo

Well, in my mind, proposal 3 is quite the same as the proposal 1, because:

  • in proposal 1, the Go code must know the actual Python API details and generate code to call them.
  • in proposal 3, Python API must know the Go side IR details and extract parameters from IR to do the actual training.
  • when we want to change the IR struct or the Python API, both sides should be updated.

typhoonzero avatar May 27 '20 13:05 typhoonzero

Well, in my mind, proposal 3 is quite the same as the proposal 1, because:

  • in proposal 1, the Go code must know the actual Python API details and generate code to call them.
  • in proposal 3, Python API must know the Go side IR details and extract parameters from IR to do the actual training.

In the proposed architecture, the IR is not "Go side", it's a formal protocol between the Parser (Go side) and the Runtime (Python side), and that's why #2335 prefers the protobuf way.

  • when we want to change the IR struct or the Python API, both sides should be updated.
  1. The Python API is based on the module sqlflow.execute, the Go part in the proposed architecture is based on the module sqlflow.execute too. We don't have to modify the Go part when changing the Python API.

  2. A change to IR struct usually implies an important update that may affect the behavior of the Programs, in such cases, we always have to modify both the parser (in Go) and the Runtime (in Python). The proposal 3 helps this process by decoupling the necessary work, i.e. modifying IR, modifying the parser (Go), modifying the runtime (Python) can be done by different people at different time.

shendiaomo avatar May 27 '20 14:05 shendiaomo

In the proposed architecture, the IR is not "Go side", it's a formal protocol between the Parser (Go side) and the Runtime (Python side), and that's why #2335 prefers the protobuf way.

The protocol can be a protobuf, and also can be the Python API functions. The question is does the protocol often changes?

  1. Does the IR changes often? No.
  2. Does the refactored Python API changes often? No, and must not. We expose Python APIs to users, if the API changes often, it's a terrible user experience.
  3. Does the current sqlflow_submitter Python API changes often? Yes.

typhoonzero avatar May 27 '20 15:05 typhoonzero

As I see, here are two things:

  1. Should we use ir as a protocol between Go and Python. Yes because, proposal 2 and 3 actually doing the same thing. According to single-responsibility principle, I prefer proposal 3. Go only do abstract syntax analysis and generate ir. Python implement the semantics of the ir.
  2. Should we expose the sqlflow.inner.train(ir, engine="pai") to end-user? No, I think there should be a more high-level api, which contains train/predict/explain functions like proposal 2.

lhw362950217 avatar May 28 '20 07:05 lhw362950217

In the proposed architecture, the IR is not "Go side", it's a formal protocol between the Parser (Go side) and the Runtime (Python side), and that's why #2335 prefers the protobuf way.

The protocol can be a protobuf, and also can be the Python API functions. The question is does the protocol often changes?

  1. Does the IR changes often? No.

That's true. IR will not change often.

  1. Does the refactored Python API changes often? No, and must not. We expose Python APIs to users, if the API changes often, it's a terrible user experience.

This is not true. A Python API must satisfy Python users' needs and will evolve in time. A protocol should satisfy the modules' needs and is relatively stable. An internal protocol is an implementation detail and doesn't have to rely on Python users' requirements. Moreover, the effort needed to design a well-defined API and the effort needed to design a well-defined internal protocol are totally different.

  1. Does the current sqlflow_submitter Python API changes often? Yes.

That' true.

shendiaomo avatar May 28 '20 09:05 shendiaomo