Skip to content

Comments

feat(evaluation): add HPSMetric#546

Open
davidberenstein1957 wants to merge 9 commits intomainfrom
feat/metric-hps
Open

feat(evaluation): add HPSMetric#546
davidberenstein1957 wants to merge 9 commits intomainfrom
feat/metric-hps

Conversation

@davidberenstein1957
Copy link
Member

Add HPSMetric using hpsv2 for human preference scoring.

- Create metric_hps.py using HPSv2 library
- Uses ViT-H-14 model with laion2B-s32B-b79K
- Add hpsv2 to evaluation dependencies
- Register HPSMetric in __init__.py
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

# Process the prompt
text = self.tokenizer([prompt]).to(device=self.device, non_blocking=True)
# Calculate the HPS
with torch.amp.autocast(device_type=self.device.type):
Copy link

Choose a reason for hiding this comment

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

String device has no .type attribute, causing crash

High Severity

self.device is assigned from set_to_best_available_device(), which returns a str (e.g., "cuda:0"). At line 172, self.device.type is called, but Python strings have no .type attribute — this raises an AttributeError every time update is invoked. It likely needs torch.device(self.device).type or a string split to extract the device type.

Additional Locations (1)

Fix in Cursor Fix in Web

import hpsv2
except ImportError:
pruna_logger.error("hpsv2 is not installed. Install with: pip install hpsv2")
raise
Copy link

Choose a reason for hiding this comment

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

Redundant try-import after top-level hpsv2 imports

Low Severity

The try: import hpsv2 guard inside __init__ is dead code. The module already imports from hpsv2.src.open_clip import ... and from hpsv2.utils import ... at the top level, so a missing hpsv2 package would cause an ImportError at module load time, long before __init__ is reached.

Additional Locations (1)

Fix in Cursor Fix in Web

text = self.tokenizer([prompt]).to(device=self.device, non_blocking=True)
# Calculate the HPS
with torch.amp.autocast(device_type=self.device.type):
outputs = model(image_tensor, text)
Copy link

Choose a reason for hiding this comment

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

Parameter outputs shadowed by model result in loop

Low Severity

The local variable outputs on line 173 reuses the name of the method parameter outputs. While it doesn't cause a functional bug currently (since the parameter isn't read after this point), the shadowing is confusing and fragile — any future refactoring that accesses the original outputs after the loop would silently get the wrong value.

Fix in Cursor Fix in Web

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.

1 participant