[Discussion] structural refactor to make the source more like 'idiomatic go'

[ddysher]

Right now, we have the components running (excluding modeldb):

• dlk-manager
• vizier-core
• vizier-db
• vizier-suggestion-[algorithm]

These are the binaries I've found after briefly scanning the code structure:

.
├── cli
│   ├── Dockerfile
│   └── main.go
├── dlk
│   └── dlkmanager
│       └── dlkmanager.go
├── manager
│   └── main.go
├── suggestion
│   ├── grid
│   |   └── main.go
│   └── random
│       └── main.go
└── earlystopping
    └── medianstopping
        └── main.go

Personally, I think there are two issues we can fix to improve the code base:

• component naming and source code naming is inconsistent
• structure can be improved to make it consistent with go projects in the wild :)

Here is the structure off my head:

├── cmd
│   ├── cli
│   │   └── cli.go
│   └── dlkctl
│   │   └── dlkcli.go
│   └── dlkmanager
│   │   └── dlkmanager.go
│   └── viziercore
│   |   └── viziercore.go
│   └── earlystopping
│       └── medianstopping.go
├── pkg
│   └── apis
|       └── v1alpha1
|           └── api.proto
│   └── db
│   └── dlk
│   └── mock
│   └── vizier
│   └── suggestion
│   └── earlystopping
├── suggestion
|   └── random
|   └── grid
├── docs
├── manifests
│   └── conf
│   └── dlk
│   └── modeldb
│   └── vizier
├── hack
│   └── build.sh
│   └── deploy.sh
├── test
├── vendor

Note that since we mostly want to write suggestion service in python, it should have its own root directory. If we want to write earlystopping service in other languages as well, then we can also move it out to top-level root.

@gaocegege @YujiOshima WDYT?

/improvement enhancement
/area suggestion

[gaocegege]

🎉 SGTM

[YujiOshima]

Thank you @ddysher ! I agree with current code structure is little confusing.
Your proposal is LGTM.
I think the earlystopping services should be the same layer of suggestions.

[gaocegege]

I think the earlystopping services should be the same layer of suggestions.

Agree. I think we should put all suggestions in CLI, too.

[ddysher]

Agree. I think we should put all suggestions in CLI, too.

I think @YujiOshima means we should put earlystopping at top level, similar to suggestion service, not put suggestions under CLI?

[YujiOshima]

Yes, it means to put the ealystoppings at top level.
They should be separated from other go pkgs since they are written not only in go but in python ( maybe others ).
Is my understanding correct? @ddysher

[gaocegege]

Sorry for the misunderstanding. But why don't we put all CLIs in cmd package?

[ddysher]

I think the main reason is that they are standalone services that are very likely to be implemented in other languages (hence they are not simply CLI)

@gaocegege do you suggest we do the following?

├── cmd
│   ├── cli
│   │   └── cli.go
│   └── dlkctl
│   │   └── dlkcli.go
│   └── dlkmanager
│   │   └── dlkmanager.go
│   └── viziercore
│   |   └── viziercore.go
│   └── earlystopping
│       └── medianstopping
│              └── medianstopping.go
│   └── suggestions
│       └── random
│              └── random.go
│       └── bayesoptimization
│              └── bayesoptimization.py
│              └── main.py
├── pkg

[gaocegege]

Yeah, I think so.

[gaocegege]

@ddysher @YujiOshima Do you think it is appropriate? I could work on the issue.

[YujiOshima]

@gaocegege Thanks! I think it is also a good way.

[YujiOshima]

/assign @gaocegege

[ddysher]

@gaocegege Thanks!

As much as I want to fix it, I personally do not have time in near future to work on it :( Glad that you can take on it!

[gaocegege]

Yeah, I know. 😄

But if you want to fix it, I can do other tasks.

WDYT?

[ddysher]

u can go ahead 😃

…

On Fri, 20 Apr 2018 at 10:13 AM Ce Gao ***@***.***> wrote: Yeah, I know. 😄 But if you want to fix it, I can do other tasks. WDYT? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#46 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACFwAXbJDEpbNUVZ4n9zuYGPpWahUpouks5tqURmgaJpZM4TWAon> .

[gaocegege]

@YujiOshima Will we keep dlk for a long time?

[YujiOshima]

@gaocegege No. It's a POC.
How about migrating it to other worker interfaces and refining the role of worker interfaces as below?

• Tensorflow, pytorch.. operator worker interface: Support distributed learning task.
• Kubernetes worker interface: For frameworks not supported by kubeflow operator. Only manage single machine task.

[gaocegege]

I think it is a separate issue, while I agree your idea 😄 I will open an new issue for it.

[jlewi]

@gaocegege @YujiOshima Update? What do we want to do for 0.3?

[ddysher]

I believe this is done? @gaocegege

[gaocegege]

We could close the issue.