Skip to content

Fix bug in render_network - #2

Open
feynmanliang wants to merge 3 commits into
msultan:masterfrom
feynmanliang:patch-1
Open

Fix bug in render_network#2
feynmanliang wants to merge 3 commits into
msultan:masterfrom
feynmanliang:patch-1

Conversation

@feynmanliang

@feynmanliang feynmanliang commented Sep 12, 2018

Copy link
Copy Markdown

L121 makes it so that index is never 0


This change is Reviewable

@msultan

msultan commented Sep 12, 2018

Copy link
Copy Markdown
Owner

ahh good catch with the index, I think it ended up not too much of an issue because I switched towards called in input features layer (cell number [65] ) "l0" instead of "f0".


weights = ','.join(map(str,lp.weight[i].data.tolist()))
bias =','.join(map(str,lp.bias[i].data.tolist()))
try:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I am guessing you have 1 feature in the input so this was causing an issue?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I encountered this on the 2-alanine example, so there's 4 (2 sin and 2 cos).

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

hmm, weird, i will investigate this further then.

@msultan msultan Sep 12, 2018

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This code gives the correct behavior without in both instances if lp is a list but it is possible that pytorch has changed how it returns the bias when tolist() is called.

In [12]: lp=[1]

In [13]: ','.join(map(str,lp))
Out[13]: '1'

In [14]: lp=[1,2,3,4]

In [15]: ','.join(map(str,lp))
Out[15]: '1,2,3,4'

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

basically, we only need a str that can be streamed to text file for plumed

@feynmanliang feynmanliang Sep 17, 2018

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Yes, the problem I was running in to was that tolist was returning a scalar, i.e .','.join(map(str, 1)), which results in a TypeError since 1 is not iterable

@msultan

msultan commented Sep 12, 2018

Copy link
Copy Markdown
Owner

if you are done with the PR, i can merge the changes. Thanks for bringing these in, I really appreciate it.

@feynmanliang

Copy link
Copy Markdown
Author

Sure go ahead and merge, I'll send more as I work.

FYI I'm using the vde package under msmbuilder

Comment thread vde_metadynamics/render_network.py Outdated
try:
bias =','.join(map(str,lp.bias[i].data.tolist()))
except TypeError:
bias = str(lb.bias[i].data.tolist())

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this should be lp.bias[i] not lb.bias[i]

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sorry about that, fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants