Lines 248-257 of neuralnetworks.py

[Alireza Khorshidi]

Doesn't lines https://bitbucket.org/andrewpeterson/amp/src/bf4a0f4096b43867d82d58f874ecd5c98e8e8175/model/neuralnetwork.py?at=master&fileviewer=file-view-default#neuralnetwork.py-248 to https://bitbucket.org/andrewpeterson/amp/src/bf4a0f4096b43867d82d58f874ecd5c98e8e8175/model/neuralnetwork.py?at=master&fileviewer=file-view-default#neuralnetwork.py-257 apply to any regression model?

If yes, then why should we have it for each model? Couldn't we move it to the Amp class?

[andrew_peterson]

Yes and no. I think it is useful to have a get_energy method in all of our classes to facilitate the user to "play with" the model directly; e.g., if I vary component 17 of my fingerprint how does my model's energy prediction change? That seems cleaner to do with model.get_energy(fingerprint) than with something from the outer Amp class, since it's independent of the rest.

However, you make a good point on duplication of code. I think the standard way to handle methods that will show up in every implementation of a model is to make a parent class Model that has this and any other standard methods; then NeuralNetwork, etc., would be derivatives of this model. E.g., in model/__init__.py:

class Model(object):
    ...
    def get_energy(self, fingerprint):
        ...

and in model/neuralnetwork.py:

class NeuralNetwork(Model):
    ...

[Alireza Khorshidi]

Good idea. I just did that. Should we move other methods common between different models to the Model class as well?

Methods like

log(): https://bitbucket.org/andrewpeterson/amp/src/bf4a0f4096b43867d82d58f874ecd5c98e8e8175/model/neuralnetwork.py?at=master&fileviewer=file-view-default#neuralnetwork.py-110

log(): https://bitbucket.org/andrewpeterson/amp/src/bf4a0f4096b43867d82d58f874ecd5c98e8e8175/model/neuralnetwork.py?at=master&fileviewer=file-view-default#neuralnetwork.py-120

tostring(): https://bitbucket.org/andrewpeterson/amp/src/bf4a0f4096b43867d82d58f874ecd5c98e8e8175/model/neuralnetwork.py?at=master&fileviewer=file-view-default#neuralnetwork.py-123

get_loss(): https://bitbucket.org/andrewpeterson/amp/src/bf4a0f4096b43867d82d58f874ecd5c98e8e8175/model/neuralnetwork.py?at=master&fileviewer=file-view-default#neuralnetwork.py-215

lossfunction(): https://bitbucket.org/andrewpeterson/amp/src/bf4a0f4096b43867d82d58f874ecd5c98e8e8175/model/neuralnetwork.py?at=master&fileviewer=file-view-default#neuralnetwork.py-233

[andrew_peterson]

Let's think this through. Generally I think it's a good idea but we also want to be careful that we only keep a minimum standard so that people can code their own models easily.

[andrew_peterson]

(Meaning we should discuss in person if there might be any implications.)

[Alireza Khorshidi]

log and tostring methods moved to Model class.