Add FLOP estimates for operator assembly - #2018
Conversation
jeremylt
left a comment
jeremylt
left a comment
There was a problem hiding this comment.
not sure how I feel about having these tests do two different things instead of one, but their descriptions should be updated
|
it should me possible to estimate at points too |
Signed-off-by: alvaroborras <13698600+alvaroborras@users.noreply.github.com>
c2ae795 to
9f46b6c
Compare
|
Thanks for the comments, I added some changes around point estimates that I was working on. |
|
See failing CI jobs - release notes need to be updated, and there's as style fix |
Signed-off-by: alvaroborras <13698600+alvaroborras@users.noreply.github.com>
|
I’ve updated the test descriptions and comments, added the required release-note entry, and applied the style fix. For AtPoints assembly estimates, CPU backends now count the actual total number of points, while GPU backends keep the max points in element count. I've also added a regression test to cover both cases. Thank you for the feedback. |
Signed-off-by: alvaroborras <13698600+alvaroborras@users.noreply.github.com>
|
Looking really close, just a couple more small things I noticed |
|
Thanks for the feedback. I’ve updated the release note ordering, reused the existing point count helper, and added the missing documentation. |
jeremylt
left a comment
jeremylt
left a comment
There was a problem hiding this comment.
two minor things, but otherwise looks ready
| } | ||
|
|
||
| /** | ||
| @brief Estimate the FLOPs required to rebuild assembled QFunction data without changing its state |
There was a problem hiding this comment.
I'd probably say here 'based upon Function rebuild status but does not modify internal state' instead of only 'without changing internal state'
| CeedCall(CeedElemRestrictionGetMaxPointsInElement(rstr_points, &max_points)); | ||
| num_qpts_total = (CeedSize)num_elem * max_points; | ||
| } else { | ||
| CeedInt num_points; |
There was a problem hiding this comment.
minor, probably don't need this intermediate variable, honestly
Document that QFunction FLOP estimates depend on rebuild status without modifying internal state. Reuse the point-count temporary across host and device paths, follow interface error-handling conventions, and keep the CHANGELOG chronological. Signed-off-by: alvaroborras <13698600+alvaroborras@users.noreply.github.com>
c581f18 to
76da6ea
Compare
2 participants
Purpose:
Add FLOP estimates for full, diagonal, and point-block diagonal operator assembly. The estimates account for stale QFunction assembly data and report AtPoints assembly as unsupported.
Closes: #1983
LLM/GenAI Disclosure:
Codex was used to brainstorm and review the changes, but the code was written by hand.
By submitting this PR, the author certifies to its contents as described by the Developer's Certificate of Origin.
Please follow the Contributing Guidelines for all PRs.