Skip to content

Content: Introduce emulation section, move reshape examples there - #598

Merged
fdwr merged 1 commit into
webmachinelearning:mainfrom
inexorabletash:content-emulation-decomposition
Jun 13, 2024
Merged

fdwr merged 1 commit into
webmachinelearning:mainfrom
inexorabletash:content-emulation-decomposition

Conversation

@inexorabletash

@inexorabletash inexorabletash commented Mar 8, 2024 •

Copy link
Copy Markdown
Contributor

Per discussion in #551, move the squeeze, unsqueeze, and flatten examples from the reshape definition into a dedicated section.

Fixes #551


Preview | Diff

@inexorabletash

Copy link
Copy Markdown
Contributor Author

Notes:

  • The Appendices - MLOperandDataType and ArrayBufferView compatibility section is normative but now follows two non-normative sections. Can we rename and move it up?
  • This PR only tackles part of the discussion in Deprecated squeeze with emulation path is missing its own section聽#551 namely moving squeeze (etc) out of reshape. It doesn't move any example decompositions out. If we want to do that, here's an idea
    • Give the new "Operator Emulation" section two subsections: "Emulation" (what's there now) and "Decomposition"
    • Move the 23 "generically emulated" blocks into "decomposition". Reword the preambles to be "The behavior of the hardSwish operation..."
    • Leave non-normative links in a standard format, e.g. "The behavior of this operation can be generically emulated." (which links to the emulation)
    • I am worried about bloating the Table of Contents with the decompositions, though, so I'd propose we leave those out.

I don't feel strongly about any of this, it just seemed an easy issue to resolve...

@zolkis

zolkis commented Mar 9, 2024

Copy link
Copy Markdown
Collaborator

Makes sense to me.

@inexorabletash

Copy link
Copy Markdown
Contributor Author

@huningxin - can you take a look? Maybe we should merge as-is or with minor tweaks, rather than trying to "boil the ocean" with other spec structure changes? I'm happy to incorporate any suggested changes, though.

Per discussion in #551, move the squeeze, unsqueeze, and flatten
examples from the reshape definition into a dedicated section.

Fixes #551

@huningxin huningxin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@huningxin

Copy link
Copy Markdown
Contributor

@huningxin - can you take a look? Maybe we should merge as-is or with minor tweaks, rather than trying to "boil the ocean" with other spec structure changes?

+1 to merge as-is.

@fdwr , do you have any further comments?

@anssiko anssiko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Happy to see this editorial detail get addressed. Now seeing this in the spec preview helps a bunch and I agree this is a good compromise i.e. keep decompositions close to ops and for those ops that are not (anymore) part of the WebNN op set put them into this Operator Emulation section.

@inexorabletash
inexorabletash requested a review from fdwr June 13, 2024 14:11

@fdwr fdwr left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

馃憤 Thanks JB.

fdwr , do you have any further comments?

Sorry, fell down into my notification soup.

Comment thread index.bs
@fdwr
fdwr merged commit 60e6447 into webmachinelearning:main Jun 13, 2024
github-actions Bot added a commit that referenced this pull request Jun 13, 2024
SHA: 60e6447
Reason: push, by fdwr

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@inexorabletash
inexorabletash deleted the content-emulation-decomposition branch June 17, 2024 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Deprecated squeeze with emulation path is missing its own section

5 participants