Skip to content

Fix wrong parsing of model config given on command line#9281

Open
GPhilo wants to merge 1 commit intotensorflow:masterfrom
GPhilo:fix_config_override
Open

Fix wrong parsing of model config given on command line#9281
GPhilo wants to merge 1 commit intotensorflow:masterfrom
GPhilo:fix_config_override

Conversation

@GPhilo
Copy link

@GPhilo GPhilo commented Sep 22, 2020

Description

The TF2 version of the SSD TFLite model exporter allows to pass a --config_override flag with a TrainEvalPipelineConfig message to alter the given pipeline_config. However, the script calls Parse instead of Merge when handling the flags. This raises an error for any non-repeated items in the message. For example, using the value in the script's documentation, it raises:

google.protobuf.text_format.ParseError: 1:19 : Message type "object_detection.protos.TrainEvalPipelineConfig" should not have multiple "model" fields.

Simply replacing Parse with Merge (similarly to how the TF1 version of the script handles the flag) solves the problem.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Tests

Run the script with the command shown in the example usage on any pre-trained SSD models from the TF2 model zoo.

Checklist

  • I have signed the Contributor License Agreement.
  • I have read guidelines for pull request.
  • My code follows the coding guidelines.
  • I have performed a self code review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
    -> The change is minimal and self-explanatory
  • I have made corresponding changes to the documentation.
    -> The fix makes the script work as already documented
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective or that my feature works.
    -> I'm not sure how to add a test for this (I'm not familiar with the testing infrastructure and I couldn't find any example for the script or its TF1 version)

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@GPhilo GPhilo force-pushed the fix_config_override branch from 564d42c to 82ec55e Compare September 22, 2020 11:13
@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@jaeyounkim jaeyounkim added the models:research models that come under research directory label Sep 25, 2020
@jaeyounkim jaeyounkim added models:research:odapi ODAPI and removed models:research models that come under research directory labels Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants