search

coarse-graining / cgnet

learning coarse-grained force fields
BSD 3-Clause "New" or "Revised" License
57 stars 26 forks source link

added embedding option to dataset loss #141

Closed brookehus closed 4 years ago

brookehus commented 4 years ago

I also updated the example notebook with dataset_loss, moving of the scheduler step, etc.

An issue I ran into is that using dataset_loss for a vanilla cgnet involves specifying that there are no embeddings that should be passed to model.forward(). @nec4 how do you feel about this solution (and example notebook implementation)? It's definitely not scalable if we do more stuff with datasets than embeddings, but hard-coding in embedding options has been our philosophy with other stuff, so it seems consistent.

nec4 commented 4 years ago

Will take a look!

brookehus commented 4 years ago

Great idea - made the changes you suggested and fixed the example notebook. What do you think now?

nec4 commented 4 years ago

Super - I will take a look and run the tests/notebook.

nec4 commented 4 years ago

Tests and notebook run - LGTM!