Skip to content

Conversation

@LHamnett
Copy link
Contributor

Motivation

In encode_decoder.py , assertion logic is not working correctly if user modifes cfg.test_cfg and defines it in a dictionary format. See: #3011

Modification

Slight change to assertion behaviour to change assertion depending on if received test_cfg object is a dict or not.

BC-breaking (Optional)

Unsure - I believe this will not break any downstream tasks as the previous logic is still included

Use cases (Optional)

n/a

@CLAassistant
Copy link

CLAassistant commented May 14, 2023

CLA assistant check
All committers have signed the CLA.

Comment on lines 329 to 332
if isinstance(self.test_cfg.mode, dict):
assert self.test_cfg['mode'] in ['slide', 'whole']
else:
assert self.test_cfg.mode in ['slide', 'whole']
Copy link
Collaborator

@xiexinch xiexinch May 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about convert the self.test_cfg to mmengine.Config at the init function?

class EncoderDecoder:
    def __init__(...):
        ...
        self.test_cfg = Config(test_cfg)
        ...

@xiexinch xiexinch changed the title Change assertion logic inference cfg.model.test_cfg [Enhancement] Change assertion logic inference cfg.model.test_cfg Jun 16, 2023
@xiexinch xiexinch merged commit b0635ff into open-mmlab:dev-1.x Jun 19, 2023
xiexinch added a commit that referenced this pull request Jun 19, 2023
nahidnazifi87 pushed a commit to nahidnazifi87/mmsegmentation_playground that referenced this pull request Apr 5, 2024
…en-mmlab#3012)

## Motivation

In encode_decoder.py , assertion logic is not working correctly if user
modifes cfg.test_cfg and defines it in a dictionary format. See:
open-mmlab#3011

## Modification

Slight change to assertion behaviour to change assertion depending on if
received test_cfg object is a dict or not.

## BC-breaking (Optional)

Unsure - I believe this will not break any downstream tasks as the
previous logic is still included

## Use cases (Optional)

n/a

---------

Co-authored-by: xiexinch <[email protected]>
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