hopeful fix for dirty PSUs [ready for review and testing] #281
Conversation
|
MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅ |
arjunsuresh
left a comment
arjunsuresh
left a comment
There was a problem hiding this comment.
Thank you @dmiskovic-NV for the fix. LGTM
dmiskovic-NV
changed the title
|
marked as DONOTUSE, since there is some error in logic. will fix it tomorrow. |
ptd_client_server/lib/server.py
Outdated
| # in case such things are not done, peaks over 3x range will be cut off, so measured/reported power will be reported as lower than realistic | ||
|
|
||
| if self._maxAmps is not None and self._maxVolts is not None: | ||
| w = float(self._maxAmps) * float(self._maxVolts) |
There was a problem hiding this comment.
Here power factor must also be multiplied right?
There was a problem hiding this comment.
exactly. and to get that, i need to modify max_volts_amps which will break all unit tests and when doing that, i'll just either base decision on power factor alone or rather on real power
There was a problem hiding this comment.
Oh sure. I think using power factor alone should be reasonable here.
There was a problem hiding this comment.
we'd need to make number up since power factor is not 100% reliable indicator of peak. THD is better, but PTD doesn't gather that. so instead i opted for modifying the function and also grabbing power
|
Ready for review and testing |
dmiskovic-NV
changed the title
|
@nv-etcheng Could you review? |
araghun
left a comment
araghun
left a comment
There was a problem hiding this comment.
Approved per alignment in 1/31 PowerWG
|
Merging per the discussion in Power WG meeting ( Jan 31 2023) . A vote was taken to approve this PR |
|
@dmiskovic-NV , @araghun , @arjunsuresh - We need to remove this from master and can reintroduce it when new PTD comes through since this is patch work. We now have submission checker to check ranging/testing and we should not add any CF or range changes manually. Let's get all the tuning be done through PTD in ranging for 3.1 |
6 participants
As discussed in #280