fferroni / PhasedLSTM-Keras

Keras implementation of Phased LSTM [https://arxiv.org/abs/1610.09513]
MIT License
145 stars 53 forks source link

Is this compatible with tensorflow 2 ? #26

Open nk-fouque opened 3 years ago

nk-fouque commented 3 years ago

Hi, has someone made an update to that implementation to make it compatible with tensorflow 2 ? I cannot really afford to downgrade my version for other parts of my project

AutoUpdatingBSoD commented 3 years ago

Hello, PhasedLSTM-Keras user here. I'm using containerization and a custom PC for the project as needed for what I'm doing.

I don't know the scope of what you're doing, but it doesn't look like the author is good about documenting the project much at all. I would look into using Anaconda Containers (possibly docker as well) if it's at all possible (The former is exactly what I'm doing).

nk-fouque commented 3 years ago

@AutoUpdatingBSoD I'm not familiar with Anaconda containers, how can they make this code work with implementations in tensorflow 2 ?

AutoUpdatingBSoD commented 3 years ago

@AutoUpdatingBSoD I'm not familiar with Anaconda containers, how can they make this code work with implementations in tensorflow 2 ?

@nk-fouque Answer: you don't. containerized environments are, by definition, locked to specific versions of packages that are known to work with those packages. That's why Anaconda, Docker, VMs, make/cmake, etc. is so popular. You have to set up the container to work with the package versions you know work.

The reason you would do this, of course, is because the whole meme of "it works on my machine" doesn't apply, because in these cases you literally export what you need from your machine.

I'll see if I can find the link, but I remember seeing a setup tutorial for Python 3.5 and Anaconda Containers with the versions of Tensorflow and Keras PhasedLSTM-Keras demands.

The issue I have with the package, and it's why I need to stop working on it for now, is that after fixing a bunch of other problems with the library, I've found the Sequential model broken with training and testing data, as modified from example code in the Repository. Maybe I'm doing something wrong here (and that's always a possibility) but I'm not seeing much hope on the Repository.

AutoUpdatingBSoD commented 3 years ago

Hello, PhasedLSTM-Keras user here. I'm using containerization and a custom PC for the project as needed for what I'm doing.

I don't know the scope of what you're doing, but it doesn't look like the author is good about documenting the project much at all. I would look into using Anaconda Containers (possibly docker as well) if it's at all possible (The former is exactly what I'm doing).

I was half-awake when I read this and thought you were referring to other projects. What exactly are you referring to by "other parts of the project"? Are you referring to code within the same functional environment that has Dependency hell issues between TF 1 and 2? If that's the case, I'm not sure Anaconda will help you.

That said, with an anaconda container, the only problem I had running the example code was when I tried to use a Tensorflow/Keras combination higher than like 2.1.5 or something like that. It was when I modified the example that the Sequential model threw a fit.

What methods exactly are you trying to call? I suspect the issue you're describing (since I could get it working with a version of TF 2) and the issue you actually need solved aren't one and the same, but I'm probably wrong.

nk-fouque commented 3 years ago

Indeed, I cannot manage to create a sequential model, currently I get

TypeError: The added layer must be an instance of class Layer. Found: <phased_lstm_keras.PhasedLSTM.PhasedLSTM object at 0x7f1979d1f450>

But a few weeks ago I had managed to do this kind of things somehow (can't remember how) and I still had trouble feeding it data in the form of a TFRecordDataset, so knowing that, I was trying to find a new implementation altogether, one that would use the Tensorflow 2 implementations and would hopefully solve all my problems at once

AutoUpdatingBSoD commented 3 years ago

This is a long shot, but have you tried Tensorflow's Functional Model? Everywhere I'm reading is saying as a general rule, that one's better at basically everything.

I'm no ML expert, granted.

nk-fouque commented 3 years ago

Yes I have, I usually prefer that too, but the person who showed me this package had had trouble with the functional model and managed to do it with the sequential, so I tried it that way When I use the functional model, what I get is

AttributeError: 'Node' object has no attribute 'output_masks'
AutoUpdatingBSoD commented 3 years ago

How did they run the Sequential model, because it's obvious that the Sequential Model works to some extent with Keras/TF 2+ (if it didn't the example code wouldn't run).

AutoUpdatingBSoD commented 3 years ago

What I think is the real issue, for clarity, is that we're both not defining the model properly.

nk-fouque commented 3 years ago

They managed to run it but only with tf 1.15 and keras 2.1.5 (and python 3.7), not tf 2+

AutoUpdatingBSoD commented 3 years ago

Sorry, I wasn't clear. What I meant was what does the statement look like where you're actually adding the model?

I.E. The part where the program actually crashes.

AutoUpdatingBSoD commented 3 years ago

The reason I ask is because I get the same thing even with a similar setup, that's why I suspect it's something else.

nk-fouque commented 3 years ago

Sorry, I wasn't clear. What I meant was what does the statement look like where you're actually adding the model?

I.E. The part where the program actually crashes.

I'm not sure what you are asking, my code is this :

    ipt = Input(shape=(seconddim, thirddim), dtype='float32', name='input')
    x = PLSTM(128, activation='sigmoid', return_sequences=True)(ipt) # <-- This is where it crashes
    out = TimeDistributed(Dense(1, activation='sigmoid'))(x)

    model = Model(inputs=ipt, outputs=out)

    loss = Huber()
    optimizer = Adam(lr=0.0001)
    metrics = ['accuracy']

    model.compile(optimizer=optimizer, loss=loss, metrics=metrics)

I think it is pretty standard

AutoUpdatingBSoD commented 3 years ago

What I'm saying is this:

TF Version: 1.15.0
Keras Version: 2.1.5
tf.keras Version: 2.2.4-tf
WARNING:tensorflow:From C:\Users\unrea\anaconda3\envs\tf15\lib\site-packages\keras\backend\tensorflow_backend.py:68: The name tf.get_default_graph is deprecated. Please use tf.compat.v1.get_default_graph instead.

Traceback (most recent call last):
  File "c:/Users/unrea/Desktop/model.py", line 182, in <module>
    main()
  File "c:/Users/unrea/Desktop/model.py", line 152, in main
    model_PLSTM.add(PLSTM(units=32, input_shape=(X_train.shape[1], X_train.shape[2]), implementation=2))
  File "C:\Users\unrea\AppData\Roaming\Python\Python37\site-packages\tensorflow_core\python\training\tracking\base.py", line 457, in _method_wrapper
    result = method(self, *args, **kwargs)
  File "C:\Users\unrea\AppData\Roaming\Python\Python37\site-packages\tensorflow_core\python\keras\engine\sequential.py", line 157, in add
    'Found: ' + str(layer))
TypeError: The added layer must be an instance of class Layer. Found: <phased_lstm_keras.PhasedLSTM.PhasedLSTM object at 0x000001B7825F33C8>

I got a similar error message to you from modifying the source's examples to add my own Pandas dataframes, the code fails before it even gets to pandas, but my TF/Keras are exactly the same as your acquaintance.

However, recall that I did get the examples working with more recent TF/Keras

I'm saying I'm not entirely sure the versioning you're saying is the issue here. I think it's model definition.

AutoUpdatingBSoD commented 3 years ago

That said, I'm no ML expert. I'm just a silly person on the internet who happens to know how to write a little code.

AutoUpdatingBSoD commented 3 years ago

Sorry, I wasn't clear. What I meant was what does the statement look like where you're actually adding the model? I.E. The part where the program actually crashes.

I'm not sure what you are asking, my code is this :

    ipt = Input(shape=(seconddim, thirddim), dtype='float32', name='input')
    x = PLSTM(128, activation='sigmoid', return_sequences=True)(ipt) # <-- This is where it crashes
    out = TimeDistributed(Dense(1, activation='sigmoid'))(x)

    model = Model(inputs=ipt, outputs=out)

    loss = Huber()
    optimizer = Adam(lr=0.0001)
    metrics = ['accuracy']

    model.compile(optimizer=optimizer, loss=loss, metrics=metrics)

I think it is pretty standard

I could be wrong, but this looks like TF's Functional API. Input() as opposed to Sequential() is what tells me that

@fferroni this article will give you insight as to why I think that, there are several more just like it: https://hanifi.medium.com/sequential-api-vs-functional-api-model-in-keras-266823d7cd5e

nk-fouque commented 3 years ago

Yes yes, this one is the functional one, I thought that was what you wanted to see my sequential version is this :

    model = Sequential()
    model.add(Input(shape=(seconddim, thirddim), dtype='float32', name='input'))
    model.add(PLSTM(128, activation='sigmoid', return_sequences=True))
    model.add(TimeDistributed(Dense(1, activation='sigmoid')))

    loss = Huber()
    optimizer = Adam(lr=0.0001)
    metrics = ['accuracy']

    model.compile(optimizer=optimizer, loss=loss, metrics=metrics)
    return model
AutoUpdatingBSoD commented 3 years ago

It's not exactly the same as my model, that's for sure. There's a couple similarities.

When I have time (which probably won't happen for a while) I'll have to look into why the sequential model is misbehaving. I suspect several things are wrong here. Starting with what you said here:

But a few weeks ago I had managed to do this kind of things somehow (can't remember how)

A number of things could have changed since then. Bad update to some package comes to mind (That's why containerization is important!!!)

You were clearly able to get it working at one point, regardless. Can you do us all a huge favor and document your steps? There is technically another version of this package but it's in even worse shape than this one ATM.

Anything you can remember will help.

AutoUpdatingBSoD commented 3 years ago

Even if you don't manage to get anything working, the fact that you tried something and failed means we (the internet) know not to try that thing again. And this thread probably constitutes more actual documentation than what's on any of the project pages (skip the problem threads)

AutoUpdatingBSoD commented 3 years ago

@nk-fouque when I get time, what I'll probably do is fork the code and try to get the library working for real. As it stands I can't do that ATM because I have other priorities but if it's at all possible to document anything you find out about this library here (even if you can't retrace your steps), tha'td be a huge help ok? Thanks in advance!

nk-fouque commented 3 years ago

Ah I finally found what made it almost work the first time, the secret was in the imports This package is built by importing "pure" keras functions (i.e. not from tensorflow.keras) so the other layers must be imported from keras.layers and not tensorflow.keras.layers too

from phased_lstm_keras.PhasedLSTM import PhasedLSTM as PLSTM
from keras.layers import Input, TimeDistributed, Dense
from keras.models import Model
from tensorflow.keras.losses import Huber
from keras.optimizers import Adam

But now I do have a problem that comes from Dataset and tensorflow versioning :

Traceback (most recent call last):
  File "implementation.py", line 78, in <module>
    model.fit(X_y_train, epochs=20)
  File "/home/nfouque/Documents/Thesis/Implementation/venv/lib/python3.7/site-packages/keras/engine/training.py", line 952, in fit
    batch_size=batch_size)
  File "/home/nfouque/Documents/Thesis/Implementation/venv/lib/python3.7/site-packages/keras/engine/training.py", line 751, in _standardize_user_data
    exception_prefix='input')
  File "/home/nfouque/Documents/Thesis/Implementation/venv/lib/python3.7/site-packages/keras/engine/training_utils.py", line 92, in standardize_input_data
    data = [standardize_single_array(x) for x in data]
  File "/home/nfouque/Documents/Thesis/Implementation/venv/lib/python3.7/site-packages/keras/engine/training_utils.py", line 92, in <listcomp>
    data = [standardize_single_array(x) for x in data]
  File "/home/nfouque/Documents/Thesis/Implementation/venv/lib/python3.7/site-packages/keras/engine/training_utils.py", line 27, in standardize_single_array
    elif x.ndim == 1:
AttributeError: 'DatasetV1Adapter' object has no attribute 'ndim'
dannyneil commented 3 years ago

For those interested, I took a stab yesterday at porting Phased LSTM to Pytorch. It does succeed in training on the basic sin wave classification task from the paper, using asychronously-sampled data points, and it's a fairly minor modification on the LSTM cell. If there's interest or pull requests, happy to do more to help out. Obviously not the same as having it in TF 2 or Keras, but hopefully still helpful.

https://github.com/dannyneil/pytorch_plstm

AutoUpdatingBSoD commented 3 years ago

@dannyneil I'm going to have to take a look at this in when I have time. I only used this library because it was the only thing available that was somewhat working at the time.

No attacks or criticism intended to be directed at the repository owners of PhasedLSTM-Keras either. I get that other priorities come in the way of doing things and I myself have some projects I need to get further ahead on developing.

It was more so an observation of the fact that there are some apparently simple-to-solve issues on the repository dating back to 2017 that haven't been closed yet.

AutoUpdatingBSoD commented 3 years ago

Ah I finally found what made it almost work the first time, the secret was in the imports This package is built by importing "pure" keras functions (i.e. not from tensorflow.keras) so the other layers must be imported from keras.layers and not tensorflow.keras.layers too

from phased_lstm_keras.PhasedLSTM import PhasedLSTM as PLSTM
from keras.layers import Input, TimeDistributed, Dense
from keras.models import Model
from tensorflow.keras.losses import Huber
from keras.optimizers import Adam

But now I do have a problem that comes from Dataset and tensorflow versioning :

Traceback (most recent call last):
  File "implementation.py", line 78, in <module>
    model.fit(X_y_train, epochs=20)
  File "/home/nfouque/Documents/Thesis/Implementation/venv/lib/python3.7/site-packages/keras/engine/training.py", line 952, in fit
    batch_size=batch_size)
  File "/home/nfouque/Documents/Thesis/Implementation/venv/lib/python3.7/site-packages/keras/engine/training.py", line 751, in _standardize_user_data
    exception_prefix='input')
  File "/home/nfouque/Documents/Thesis/Implementation/venv/lib/python3.7/site-packages/keras/engine/training_utils.py", line 92, in standardize_input_data
    data = [standardize_single_array(x) for x in data]
  File "/home/nfouque/Documents/Thesis/Implementation/venv/lib/python3.7/site-packages/keras/engine/training_utils.py", line 92, in <listcomp>
    data = [standardize_single_array(x) for x in data]
  File "/home/nfouque/Documents/Thesis/Implementation/venv/lib/python3.7/site-packages/keras/engine/training_utils.py", line 27, in standardize_single_array
    elif x.ndim == 1:
AttributeError: 'DatasetV1Adapter' object has no attribute 'ndim'

Thank you for doing that and reminding me of one of the issues I was facing. I had forgotten about this myself. I had also gotten my code further ahead with using pure Keras, but I was having issues with Keras's implementation accepting my pandas dataframe shape. It was fine with the fact that pandas was a parameter, so that's not the issue.

In any case, I'm probably going to take up @dannyneil if I consider writing code with this. No offense or attacks directed or intended towared the repo holder, I just don't feel now's the best time to develop for PhasedLSTM-Keras