Begin migration of enqueue_function to enqueue_function_checked
#127
Conversation
ehsanmok
left a comment
ehsanmok
left a comment
There was a problem hiding this comment.
Thank you Brad! Appreciating the changes. One comment on the layout tensor pattern.
| alias dtype = DType.float32 | ||
| alias layout = Layout.row_major(HEIGHT, WIDTH) | ||
|
|
||
| fn kernel[dtype: DType, layout: Layout](tensor: LayoutTensor[mut=True, dtype, layout]): |
There was a problem hiding this comment.
This change isn't required afaik and the existing pattern is more succinct than using the explicit MutableAnyOrigin. Wdyt?
book/src/puzzle_04/intro.mojo
Outdated
| alias layout = Layout.row_major(HEIGHT, WIDTH) | ||
|
|
||
| fn kernel[dtype: DType, layout: Layout](tensor: LayoutTensor[mut=True, dtype, layout]): | ||
| fn kernel[dtype: DType, layout: Layout](tensor: LayoutTensor[dtype, layout, MutableAnyOrigin]): |
There was a problem hiding this comment.
| fn kernel[dtype: DType, layout: Layout](tensor: LayoutTensor[dtype, layout, MutableAnyOrigin]): | |
| fn kernel[dtype: DType, layout: Layout](tensor: LayoutTensor[dtype, layout, MutAnyOrigin]): |
Here and elsewhere due to rename from @NathanSWard today
ehsanmok
left a comment
ehsanmok
left a comment
There was a problem hiding this comment.
Pedagogically please use the mut=True pattern as the use of origin adds more cognitive load.
dunnoconnor
left a comment
dunnoconnor
left a comment
There was a problem hiding this comment.
Great. These changes are consistent with warning messages and with the 25.7 Nightly Library changes: https://docs.modular.com/mojo/changelog/#25-7-library-changes
dunnoconnor
left a comment
dunnoconnor
left a comment
There was a problem hiding this comment.
This is consistent with guidance in the changelog https://docs.modular.com/mojo/changelog/#25-7-library-changes
BradLarson
dismissed
ehsanmok’s stale review
We will improve upon the ergonomics in a follow-on, but we need to merge this to make progress towards getting the repository to build on the upcoming nightly.
|
The nightly release is no longer tied to the puzzles. Please fix the origin issue I raised earlier as a fast follow. |
|
Checked with the team, and it unfortunately appears that simply specifying There may be a different way we can approach this, but unfortunately at present a simple |
4 participants
We are beginning a transition from
enqueue_functiontoenqueue_function_checked, on path to eventually replacing the originalenqueue_functionwith a new typechecked version. However, there are some compiler issues that need to be resolved before the newenqueue_functionis ready, so in the meantime we are doing a stepwise migration toenqueue_function_checkedto gain the benefits of typechecking on GPU functions.enqueue_function_checkedrequires some code modifications, so this begins that process across the puzzles. Not all puzzles are updated, because some require a bit more manual care to transition over.