fix text preprocessing for inference model

[xwli-chelsea]

Description

In https://github.com/linkedin/detext/pull/28 the filter_window_sizes override was removed. The param passing to process_text is inconsistent for none-CNN models:

• process_text: https://github.com/linkedin/detext/blob/master/src/detext/train/data_fn.py#L151
• training call (correct): https://github.com/linkedin/detext/blob/master/src/detext/train/train.py#L60
• inference model call (incorrect after #28 ): https://github.com/linkedin/detext/blob/master/src/detext/train/train_helper.py#L121

This inconsistency causes problems with the inference savedmodel models.

This pr fixes the logic for filter_window_sizes in the serving_input_fn.

Fixes # (issue) 474: internal issue reported for inference model

Type of change

Please delete options that are not relevant.

• [x] Bug fix (non-breaking change which fixes an issue)
• [ ] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

List all changes

Update logic in train_helper, where the helper functions are used in serving_input_fn. This sets the filter_window_sizes for cnn only. Code search on this logic

Testing

pytest local training run, and tested inference model.

Test Configuration:

• Firmware version:
• Hardware:
• Toolchain:
• SDK:

Checklist

• [ ] My code follows the style guidelines of this project
• [ ] I have performed a self-review of my own code
• [ ] I have commented my code, particularly in hard-to-understand areas
• [ ] I have made corresponding changes to the documentation
• [ ] My changes generate no new warnings
• [ ] I have added tests that prove my fix is effective or that my feature works
• [ ] New and existing unit tests pass locally with my changes
• [ ] Any dependent changes have been merged and published in downstream modules

[StarWang]

Thanks for spotting this. Nice finding! To prevent this from happening in the future, it'd be better that we put filter_window_sizes as 0 in the source, e.g., in method extend_hparams. Otherwise, we'll need to exhaustively find all the end point of filter_window_sizes and fix cases like this.

[xwli-chelsea]

Thanks for spotting this. Nice finding! To prevent this from happening in the future, it'd be better that we put filter_window_sizes as 0 in the source, e.g., in method extend_hparams. Otherwise, we'll need to exhaustively find all the end point of filter_window_sizes and fix cases like this.

Not sure why it was removed but it seems we can just keep the logic in extend_hparams. Let me add it back.

[xwli-chelsea]

Thanks for fixing this :)

Is there any chance that we can put a test case for inference to test it?

I thought about what tests we can add. In the case of this issue, what we need is to make sure the params passed to process_text should be the same as during training. So it's not suitable for a unit test.

But you brought a good point😊. We could probably add (maybe in another pr) some comprehensive integration tests to check if training input_fn and serving_input_fn have the same expected behaviors. I'll create an issue to track. Thanks for the comment!

[xwli-chelsea]

Thanks for fixing this :)

Is there any chance that we can put a test case for inference to test it?

I thought about what tests we can add. In the case of this issue, what we need is to make sure the params passed to process_text should be the same as during training. So it's not suitable for a unit test.

But you brought a good point😊. We could probably add (maybe in another pr) some comprehensive integration tests to check if training input_fn and serving_input_fn have the same expected behaviors. I'll create an issue to track. Thanks for the comment!