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

fix incorrect price calculation in sf orders ls #64

Merged
merged 1 commit into from
Feb 7, 2025
Merged

Conversation

Sladuca
Copy link
Contributor

@Sladuca Sladuca commented Feb 7, 2025

No description provided.

Copy link

semanticdiff-com bot commented Feb 7, 2025

Review changes with  SemanticDiff

Changed Files
File Status
  src/lib/orders/OrderDisplay.tsx  56% smaller

@Sladuca Sladuca merged commit cffc4ab into main Feb 7, 2025
1 check passed
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Fixed price calculation logic in OrderDisplay.tsx, focusing on price per GPU hour calculations for order listings.

  • Modified price calculation in orderDetails() to divide price by (quantity * duration * GPUs) instead of multiplying first
  • Potential bug: Inconsistency between regular price and executed price calculations that could lead to incorrect values
  • Edge case risk: Division by zero possible when duration is 0, currently handled by defaulting to 1 hour
  • Consider using helper functions from src/helpers/price.ts for consistent price calculations across the codebase

1 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines 12 to +13
const pricePerGPUHour =
(order.price * order.quantity) / GPUS_PER_NODE / durationInHours / 100;
order.price / (order.quantity * durationInHours * GPUS_PER_NODE) / 100;
Copy link

Choose a reason for hiding this comment

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

logic: The price calculation for regular and executed prices are inconsistent. Regular price divides first while executed price multiplies first. This will give different results.

@sigmachirality sigmachirality deleted the seb/fix-price-calc branch February 15, 2025 20:19
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