I am following this tutorial .

My mission is to convert an English sentence to a German sentence using Bahdanau Attention.

Summary of the Code

I first took the whole English and German sentence in input_english_sent and input_german_sent respectively. Applied an Embedding Layer on both of them. Passed the input_english_sent, i.e. the whole English sentence, to encoder. And then, I have used a for loop, for implementing decoder with Bahdanau Attention. Inside the for loop, I first take the i word of the whole German Sentence, and declare it as the input to the decoder, i.e. in the first run of the loop, the input to the decoder will be the first word (which is 'start_seq_'), and in the next run, the input would be the second word and so on. And, then in the next lines of the for loop, I compute the Attention and concatenate it with the input of the decoder. And then, in the last line of the for loop, I append the decoder output to an array named final_stage, so that I could apply the Dense() Layer, to predict all the words altogether.


The code works fine, and it does not produces any error. I just wanna be sure that my implementation is really Bahdanau Attention implementation.

The Code inside the for loop has to be checked, as that is the part that implements the Bahdanau attention.


You can ignore all the tf.expand_dims() and tf.reshape() layers as they are just there to avoid any errors regarding shapes.

This is my code:

input_english_sent = Input(shape=(english_max_len,))
input_german_sent = Input(shape=(german_max_len,))

encoder_inputs = Embedding(english_vocab_size, 256)(input_english_sent)
decoder_inputs = Embedding(german_vocab_size, 256)(input_german_sent)

encoder = LSTM(256, return_sequences=True, return_state=True)
encoder_outputs, encoder_state_h, encoder_state_c = encoder(encoder_inputs)

decoder_state_h = encoder_state_h
decoder = LSTM(256, return_sequences=True, return_state=True)
attention_layer1 = Dense(10)
attention_layer2 = Dense(10)
final_attention_layer = Dense(1)

final_stage = []

for i in range(german_max_len):
    decoder_input = tf.expand_dims(decoder_inputs[:,i], axis=1)
    attention_weights = Activation('tanh')(attention_layer1(encoder_outputs) + tf.expand_dims(attention_layer2(decoder_state_h), axis=1))
    attention_weights = Activation('softmax')(final_attention_layer(attention_weights))

    Context_Vector = encoder_outputs * attention_weights
    decoder_input = Concatenate(axis=1)([decoder_input, Context_Vector])

    decoder_outputs, decoder_state_h, _ = decoder(decoder_input)

output = Dense(german_vocab_size, activation='softmax')(tf.expand_dims(final_stage, axis=1))
output = tf.reshape(output, (-1, german_max_len, german_vocab_size))

model = Model(inputs=[input_english_sent, input_german_sent], outputs=output)

The Entire Code

import string
import re
from tensorflow.keras.preprocessing.text import Tokenizer
from tensorflow.keras.preprocessing.sequence import pad_sequences
from tensorflow.keras.models import Model
from tensorflow.keras.layers import Embedding, LSTM, RepeatVector, Dense, Dropout, BatchNormalization, TimeDistributed, AdditiveAttention, Input, Concatenate, Flatten
from tensorflow.keras.layers import Activation, LayerNormalization
from numpy import array
from tensorflow.keras.utils import plot_model
from sklearn.utils import shuffle
import time
import tensorflow as tf
import numpy as np

def load_data(filename):
    file = open(filename, 'r')
    text = file.read()
    return text

def to_lines(text):
    return text.split('\n')

def clean_data(pair, lang):
    if lang == 'ger_input':
        pair = 'start_seq_ ' + pair
    if lang == 'ger_output':
        pair = pair + ' end_seq_'
    if lang == 'ger':
        pair = 'start_seq_ ' + pair + ' end_seq_'

    re_print = re.compile('[^%s]' % re.escape(string.printable))
    table = str.maketrans('', '', string.punctuation)
    tokens = [token.translate(table) for token in pair.split()]
    tokens = [token.lower() for token in tokens]
    tokens = [re_print.sub('', token) for token in tokens]
    tokens = [token for token in tokens if token.isalpha()]
    return tokens

lines = to_lines(load_data('/content/drive/My Drive/deu.txt'))

english_pair = []
german_pair = []
german_pair_input = []
german_pair_output = []
for line in lines:
    if line != '':
        pairs = line.split('\t')
        english_pair.append(clean_data(pairs[0], 'eng'))
        german_pair.append(clean_data(pairs[1], 'ger'))
        german_pair_input.append(clean_data(pairs[1], 'ger_input'))
        german_pair_output.append(clean_data(pairs[1], 'ger_output'))

english_pair = array(english_pair)
german_pair = array(german_pair)
german_pair_input = array(german_pair_input)
german_pair_output = array(german_pair_output)

english_pair, german_pair, german_pair_input, german_pair_output = english_pair[-10000:], german_pair[-10000:], german_pair_input[-10000:], german_pair_output[-10000:]

def create_tokenizer(data):
    tokenizer = Tokenizer()
    return tokenizer

def max_len(lines):
    length = []
    for line in lines:
    return max(length)

english_tokenizer = create_tokenizer(english_pair)
german_tokenizer = create_tokenizer(german_pair)

english_vocab_size = len(english_tokenizer.word_index) + 1
german_vocab_size = len(german_tokenizer.word_index) + 1

english_max_len = max_len(english_pair)
german_max_len = max_len(german_pair)

def create_sequences(sequences, tokenizer, max_len):
    sequences = tokenizer.texts_to_sequences(sequences)
    sequences = pad_sequences(sequences, maxlen=max_len, padding='post')
    return sequences

X1 = create_sequences(english_pair, english_tokenizer, english_max_len)
X2 = create_sequences(german_pair_input, german_tokenizer, german_max_len)
Y = create_sequences(german_pair_output, german_tokenizer, german_max_len)

X1, X2, Y = shuffle(X1, X2, Y)

train_x1, train_x2, train_y = X1[:7000], X2[:7000], Y[:7000]
test_x1, test_x2, test_y = X1[7000:], X2[7000:], Y[7000:]

train_y, test_y = train_y.reshape(7000, german_max_len, 1), test_y.reshape(3000, german_max_len, 1)

And then I implement the Model. The code of implementing the Model is above in the Code Section

And then I compile the model and train it.

model.compile(loss='sparse_categorical_crossentropy', optimizer='adam', metrics=['acc'])

model.fit([train_x1, train_x2], train_y, epochs=10, batch_size=32, validation_data=([test_x1, test_x2], test_y))

Data can be downloaded from here

Note - I have also implemented the teacher forcing method in it

  • 2
    \$\begingroup\$ Welcome to the Code Review site. The title should state what the code does and not your concerns about the code. The code for any functions that are used in the rest of the code should be presented in the question unless they are standard library functions otherwise there is a lack of code context. The more code presented in the question the better the answers will be because we better understand what the code does. Please read How do I ask a good question. \$\endgroup\$
    – pacmaninbw
    Oct 23, 2020 at 13:15
  • \$\begingroup\$ @pacmaninbw Thanks a lot for editing my question, and making it more clearer. Sorry for the title, as this is my first time on Code Review Site. Should I add more code? As I have written the code of the whole model. \$\endgroup\$ Oct 23, 2020 at 13:34
  • 1
    \$\begingroup\$ First glance there's zero structure to your code and too many magic number/strings. Use functions and variables \$\endgroup\$
    – Coupcoup
    Oct 23, 2020 at 13:40
  • 1
    \$\begingroup\$ Generally code review is about improving code ability, checking whether the code follows a particular implementation may not be possible if no one else knows that implementation. Please check the help center like I first suggested, it should answer most of your questions. \$\endgroup\$
    – pacmaninbw
    Oct 23, 2020 at 14:00
  • 1
    \$\begingroup\$ @Mast I have added the entire code, you can see it. \$\endgroup\$ Oct 25, 2020 at 11:44

2 Answers 2


File operations


def load_data(filename):
    file = open(filename, 'r')
    text = file.read()
    return text

is such a common and simple operation that it doesn't deserve to be a function. This doesn't lend you anything that you can't already do with open(filename).read(). Even if it were to remain, it should use with instead of an explicit close(), and filename should be marked : str.

It's also not a good idea to separate load_data and to_lines. When you get an open file handle, it's already an iterator over the file's lines, and using it as such is more efficient than loading the entire file into memory. So to_lines doesn't deserve to be a function, either.

Data cleaning

lang captures two-ish things, in a somewhat awkward way:

  • What is the language? This should be a string, and should use ISO-639-1 codes ("en") which are more common than ISO-639-2 or -3 codes ("eng").
  • Is this the end of a sequence? This should be a bool.
  • Is this the start of a sequence? This should also be a bool.

Regex pre-compilation

re_print = re.compile('[^%s]' % re.escape(string.printable))

should not appear within clean_data. The entire purpose of Python exposing the compile method is to support pre-compilation, where this would be performed typically at the global scope, only once. Otherwise, there's no advantage to doing this over a direct re.sub().

Successive list formation

If you really want to separate this into multiple statements:

tokens = [token.translate(table) for token in pair.split()]
tokens = [token.lower() for token in tokens]
tokens = [re_print.sub('', token) for token in tokens]
tokens = [token for token in tokens if token.isalpha()]

you can, but this isn't the best way. To avoid re-re-recreating the list, simply

for token in pair.split():
    token = token.translate(table)
    token = token.lower()
    # ...
    yield token

Or otherwise, a two-pass approach in "fluent" style:

tokens = (
    for token in pair.split()

with a second pass for isalpha.

max on a generator

length = []
for line in lines:
return max(length)

should be

return max(len(line) for line in lines)

or even better (thanks @hjpotter92)

return len(max(lines, key=len))

No intermediate list is needed.

  • \$\begingroup\$ max(map(len, lines)) is more efficient than max(len(line) for line in lines) but a bit less readable. \$\endgroup\$
    – GZ0
    Oct 29, 2020 at 2:19
  • 1
    \$\begingroup\$ Thanks a lot for your answer. The Code inside the for loop has to be checked, as that is the part that implements the Bahdanau attention. I wrote this in the question section. All the other code that I wrote may not be the most efficient code, but it works fine. \$\endgroup\$ Oct 29, 2020 at 3:48
  • \$\begingroup\$ It's fine that it works fine. CR expects that all of your code works fine, and all code submitted is open to review. \$\endgroup\$
    – Reinderien
    Oct 29, 2020 at 19:44
  • 2
    \$\begingroup\$ @GZ0 max(lines, key=len) :) \$\endgroup\$
    – hjpotter92
    Oct 30, 2020 at 19:37
  • 1
    \$\begingroup\$ @hjpotter92 Close but not quite. With max and a key, you end up with the item whose length is maximal; we're actually looking for the maximal length. \$\endgroup\$
    – Reinderien
    Oct 30, 2020 at 19:47

Had a quick look and I can see there are some foundamental issues of the model implementation besides the problems mentioned in other answer(s) and comment(s).

  • The German sentences should not be the decoder input. They are analogous to 'labels' of training data in classification tasks, which are not available for testing & future data. Therefore it does not make sense to feed them into the model. They should only be used to calculate loss values during training and evaluation.

  • It is incorrect to apply the same decoder sequence to the inputs german_max_len times in the loop. A correct implementation should go cell by cell, feeding the output of the each cell to the next cell as part of its input. Here you need to create LSTMCells and chain them together with customized inputs, rather than utilizing an entire LSTM sequence (technically, an LSTM with sequence length = 1 can also be used as a single cell, but LSTMCell serves better for that purpose).

  • It would be better to wrap the attention-enabled decoder into a customized layer so that the code is better structured and can be reused conveniently.


Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.