Skip to content

Add candle fe #1603

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 17 commits into
base: v1.x-develop
Choose a base branch
from
Open

Add candle fe #1603

wants to merge 17 commits into from

Conversation

graham63
Copy link

No description provided.

@benson31 benson31 added enhancement python Python frontend labels Aug 31, 2020
@benson31 benson31 requested a review from timmoon10 August 31, 2020 17:29
dummy = lbann.Dummy(input_, name='dummy')

# Encoder
encode1 = lbann.FullyConnected(data,
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are repeated layers, try a simple loop/block

label = lbann.Split(input_, name='label')

# Encoder
encode1 = lbann.FullyConnected(finetunedata,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above, you can even create a "model class" that separate driver from the actual model, please see JAG or ATOM applications examples

dummy = lbann.Dummy(input_, name='dummy')

# Encoder
encode1 = lbann.FullyConnected(image,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above and possibly put this in the vision directory

gene_dropout1 = lbann.Dropout(gene_relu1,
name="gene_dropout1",
data_layout="model_parallel",
keep_prob=0.95)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please see previous comments.

keep_prob=0.95)

# Shared Weights for Drug Tracks
drug_fc1_w = lbann.Initializer()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I dont think this correct, shouldn't it be lbann.Weight(....)?

@samadejacobs
Copy link
Collaborator

Also, I suggest a comparison of existing prototext with PFE to verify correctness.

Copy link
Collaborator

@timmoon10 timmoon10 left a comment

Choose a reason for hiding this comment

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

The models look reasonable to me, aside from some correctness issue with weights. I'd like if all of the CANDLE stuff was moved into a stand-alone directory in the app directory and removed from the model zoo directory. The var_mnist model can go in the vision app.

encode1 = lbann.FullyConnected(recon_data,
name="encode1",
data_layout="model_parallel",
weights="w1",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
weights="w1",
weights=w1,


# Encoder

w1 = lbann.Initializer()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
w1 = lbann.Initializer()
w1 = lbann.Weights(initializer=lbann.GlorotUniformInitializer())

@graham63 graham63 self-assigned this Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement python Python frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants