search

vaughnbetz / COFFE

38 stars 25 forks source link

Questions about BRAM energy consumption measurement in COFFE. #60

Closed XBQ314 closed 2 months ago

XBQ314 commented 2 months ago

Thank you for your team's hard work on this project! I really appreciate the effort and dedication put into it. My name is Buqing, and I’ve been using COFFE for measuring power of bram in FPGA in professor Mohamed Abdelfattah's lab. It would be great if you could help me clarify the following question.

In 'COFFE/coffe/toplevel.py', around line 1023 - 1027, in function 'generate_sram_read_power_top_lp', there are four 'MEASURE' to measure the power of selected and unselected column during sram read. When calculating the power, you divide the energy consumption by '4 ram_frequency'. So it's the average power during the '4 ram_frequency' time range.

In 'COFFE/coffe/fpga.py', around line 7126 - 7127, when calculate the read_energy, the formula read_energy = (self.RAM.power_sram_read.power_selected + self.RAM.power_sram_read.power_unselected) * self.RAM.frequency only times the average power above (which is measured under the period of '4 * ram_frequency') by 'self.RAM.frequence'. I notice that the energy consumption is only one-fourth of the previous amount because the energy calculation is based on a single time cycle instead of four.

My question is, do I need to multiply the energy consumption reported by COFFE by four to get the real energy consumption?

XBQ314 commented 2 months ago

@vaughnbetz

vaughnbetz commented 2 months ago

Adding @sadegh68 and @StephenMoreOSU . I don't think you need to multiply the power by 4 (that would be a bug, and I don't think there is a bug). From code inspection, I think ram_frequency is misnamed; it should really be ram_period. The transient analysis at line 1023-1027 of toplevel.py appears to be simulating four RAM periods (e.g. 4 reads), presumably to average them (note that the pulses on precharge etc. are repeating every ram_frequency which is really ram_period). Hence we divide by 4ram_frequency (really 4 ram_period) to get the average current and power. Later we multiply by ram_frequency (again really ram_period) to get the energy per read.

@StephenMoreOSU @sadegh68 : if you have time, let me know if you agree. Stephen, perhaps you renamed this variable in rad-gen/COFFE3? If you print out what it is and show it is indeed a period I think it should be renamed if you haven't already renamed it.

sadegh68 commented 2 months ago

My recollection on this isn't clear enough as I coded it many years ago, but I believe Vaughn's explanation is accurate.

Sadegh

From: vaughnbetz @.> Sent: September 12, 2024 10:44 AM To: vaughnbetz/COFFE @.> Cc: Sadegh Yazdanshenas @.>; Mention @.> Subject: Re: [vaughnbetz/COFFE] Questions about BRAM energy consumption measurement in COFFE. (Issue #60)

Adding @sadegh68https://github.com/sadegh68 and @StephenMoreOSUhttps://github.com/StephenMoreOSU . I don't think you need to multiply the power by 4 (that would be a bug, and I don't think there is a bug). From code inspection, I think ram_frequency is misnamed; it should really be ram_period. The transient analysis at line 1023-1027 of toplevel.py appears to be simulating four RAM periods (e.g. 4 reads), presumably to average them (note that the pulses on precharge etc. are repeating every ram_frequency which is really ram_period). Hence we divide by 4ram_frequency (really 4 ram_period) to get the average current and power. Later we multiply by ram_frequency (again really ram_period) to get the energy per read.

@StephenMoreOSUhttps://github.com/StephenMoreOSU @sadegh68https://github.com/sadegh68 : if you have time, let me know if you agree. Stephen, perhaps you renamed this variable in rad-gen/COFFE3? If you print out what it is and show it is indeed a period I think it should be renamed if you haven't already renamed it.

— Reply to this email directly, view it on GitHubhttps://github.com/vaughnbetz/COFFE/issues/60#issuecomment-2346502724, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ADZ5TLMSTD2ERQQQDUOY6NTZWGSEFAVCNFSM6AAAAABNVATN4CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBWGUYDENZSGQ. You are receiving this because you were mentioned.Message ID: @.***>

XBQ314 commented 2 months ago

Thank you for your kind response, it has been really helpful. I actually printed the 'ram_frequency' and found that it represents a time period.

vaughnbetz commented 2 months ago

Thanks. If you want to make a PR to change the ram_frequency name and add a comment on this 4* period simulation that would be great.

XBQ314 commented 2 months ago

Thanks. If you want to make a PR to change the ram_frequency name and add a comment on this 4* period simulation that would be great.

Sure! I have made a PR already.

vaughnbetz commented 2 months ago

Closed by PR #61