Skip to content
This repository has been archived by the owner on Mar 3, 2024. It is now read-only.

Memory-Test does not evaluate a single model #6

Closed
1 task done
xuevin opened this issue Jul 18, 2019 · 4 comments
Closed
1 task done

Memory-Test does not evaluate a single model #6

xuevin opened this issue Jul 18, 2019 · 4 comments
Assignees
Labels
invalid This doesn't seem right

Comments

@xuevin
Copy link

xuevin commented Jul 18, 2019

Describe the Bug

I believe that test is intended to evaluate the Memory function for 10 batches of sentences (or the same sentence for 10 epochs).

The current implementation initializes the Model 10x instead of initializing one model and evaluating 10 batches.

Version Info

  • I'm using the latest version

Minimal Codes To Reproduce
NA

I think the recommended change is to shift the model instantiation outside of the for loop.

        for _ in range(10):
            input_data = keras.layers.Input(shape=(3, 3))
            input_length = keras.layers.Input(shape=(1,))
            output = Memory(3, 5, 3, 3)([input_data, input_length])
            model = keras.models.Model(
                inputs=[input_data, input_length],
                outputs=output,
            )
            ....
        input_data = keras.layers.Input(shape=(3, 3))
        input_length = keras.layers.Input(shape=(1,))
        output = Memory(3, 5, 3, 3)([input_data, input_length])
        model = keras.models.Model(
            inputs=[input_data, input_length],
            outputs=output,
        )
        for _ in range(10):
            ...
@xuevin xuevin added the bug Something isn't working label Jul 18, 2019
@CyberZHG
Copy link
Owner

No. This is intend to make sure the order of calculation is correct.

See Use more memory to avoid ordering issue

@CyberZHG CyberZHG added invalid This doesn't seem right and removed bug Something isn't working labels Jul 18, 2019
@xuevin xuevin closed this as completed Jul 18, 2019
@xuevin xuevin reopened this Jul 18, 2019
@xuevin
Copy link
Author

xuevin commented Jul 18, 2019

I am not following why the nested function needs to be executed 10x. Given that you are calling Memory inside the for loop, it reinitializes the state, such that the output will always be the same.

My comment is regarding why

for _ in range(10): exists

@CyberZHG
Copy link
Owner

Because before that commit, the memory block produces wrong output with a random probability. I have to make sure it always produces the expected output.

@xuevin
Copy link
Author

xuevin commented Jul 18, 2019

Thanks for clarifying. I appreciate your work!

@xuevin xuevin closed this as completed Jul 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants