Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use torchmetrics to calculate map #99

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

vlaminckaxel
Copy link

No description provided.

@henrytsui000
Copy link
Member

henrytsui000 commented Oct 11, 2024

Thank you for your contribution! Next, I will proceed with a code review for each block. Please direct push this to the INFRENCE branch.

Best regards,
Henry Tsui

@ahmadmughees
Copy link

ahmadmughees commented Oct 11, 2024

I would just like to mention that torch metrics[detection] is a wrapper around pycocotools SEE THIS

This metric supports, at the moment, two different backends for the evaluation. The default backend is "pycocotools", which either require the official pycocotools implementation or this fork of pycocotools to be installed. We recommend using the fork as it is better maintained and easily available to install via pip: pip install pycocotools. It is also this fork that will be installed if you install torchmetrics[detection]. The second backend is the faster-coco-eval implementation, which can be installed with pip install faster-coco-eval. This implementation is a maintained open-source implementation that is faster and corrects certain corner cases that the official implementation has. Our own testing has shown that the results are identical to the official implementation. Regardless of the backend we also require you to have torchvision version 0.8.0 or newer installed. Please install with pip install torchvision>=0.8 or pip install torchmetrics[detection].

@@ -134,16 +134,16 @@ def load_valid_labels(self, label_path: str, seg_data_one_img: list) -> Union[Te
def get_data(self, idx):

Choose a reason for hiding this comment

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

Could you add return types please?

@@ -134,16 +134,16 @@ def load_valid_labels(self, label_path: str, seg_data_one_img: list) -> Union[Te
def get_data(self, idx):
img_path, bboxes = self.data[idx]
img = Image.open(img_path).convert("RGB")
return img, bboxes, img_path
return img, bboxes

def get_more_data(self, num: int = 1):

Choose a reason for hiding this comment

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

Could you add return types please?

@@ -227,7 +227,19 @@ def __call__(self, target: Tensor, predict: Tuple[Tensor]) -> Tuple[Tensor, Tens
2. Select the targets
2. Noramlize the class probilities of targets

Choose a reason for hiding this comment

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

Suggested change
2. Noramlize the class probilities of targets
2. Normalize the class probabilities of targets

ap_table.add_row(f"{epoch: 3d}", ap_name, f"{ap_color}{ap_value:.2f}", ar_name, f"{ar_color}{ar_value:.2f}")

return ap_table, this_ap
def format_prediction(prediction: torch.Tensor) -> dict:

Choose a reason for hiding this comment

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

I think for better typing you should better annotate the return type here. Something like below (not exact) to provide at least a little bit better IntelliSense to any contributors:

Suggested change
def format_prediction(prediction: torch.Tensor) -> dict:
def format_prediction(prediction: torch.Tensor) -> Dict[str, List[int]]:

def format_prediction(prediction: torch.Tensor) -> dict:
"""
Format the prediction output to a dictionary.
Args:

Choose a reason for hiding this comment

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

Suggested change
Args:
Args:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants