Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New package: Reactant v0.1.0 #106843

Merged
merged 1 commit into from
May 18, 2024

Conversation

JuliaRegistrator
Copy link
Contributor

@JuliaRegistrator JuliaRegistrator commented May 15, 2024

Copy link
Contributor

github-actions bot commented May 15, 2024

Your new package pull request met all of the guidelines for auto-merging and is scheduled to be merged when the mandatory waiting period (3 days) has elapsed.

Since you are registering a new package, please make sure that you have read the package naming guidelines: https://pkgdocs.julialang.org/v1/creating-packages/#Package-naming-guidelines


If you want to prevent this pull request from being auto-merged, simply leave a comment. If you want to post a comment without blocking auto-merging, you must include the text [noblock] in your comment. You can edit blocking comments, adding [noblock] to them in order to unblock auto-merging.

@goerz
Copy link
Member

goerz commented May 15, 2024

The package is also missing documentation [noblock]

@wsmoses
Copy link

wsmoses commented May 15, 2024

[noblock]
Yeah doc construction is failing until we get a bump on yggdrassil for reactant_jll to 0.0.6 since doc CI doesn't have a GPU

@goerz
Copy link
Member

goerz commented May 15, 2024

Some description, context, and usage example in the README would already go a long way! 😉

[noblock]

@wsmoses
Copy link

wsmoses commented May 15, 2024

[noblock]
Presently the readme is intentionally bare as end users should not use the package. I'm registering it presently to make it easier for developers to build it and perform smoke tests in meta packages to see what needs to be built (e.g. it provides functionality for other packages in the ecosystem).

You're welcome to browse our test suite for example usage, including https://github.com/EnzymeAD/Reactant.jl/blob/main/test/basic.jl and https://github.com/EnzymeAD/Reactant.jl/blob/main/test/nn.jl.

@wsmoses
Copy link

wsmoses commented May 15, 2024

@goerz as I'm realizing that one needs to include [noblock] for this to get unstuck once I finish getting our docs to build, can you add that to your comments?

internal developer documentation will be online as soon as yggdrassil merges a new jll (JuliaPackaging/Yggdrasil#8676) and we adapt docs-CI to not require a GPU.

@goerz
Copy link
Member

goerz commented May 15, 2024

Presently the readme is intentionally bare as end users should not use the package.

Huh. Is that a temporary situation? Because for a non-user-facing package, I'm not sure if I'm comfortable with "Reactant". That's the kind of nice whimsical package name that would make a nice pun in the context of Enzyme. But if it's just for internal use, such memorable names would be better kept open for other projects that get some mileage out of them, IMO.

It also doesn't seem to me that empty READMEs are ever okay, even from seasoned developers or well-established orgs like Enzyme. Maybe especially not: the newcomers we tell to add some minimal information to their README would rightfully argue "these high-profile orgs get away with registering undocumented packages". So I'm very much in favor of keeping everyone to the same (very minimal) standards. Which doesn't include a fully built documentation, but it does include a minimal README.

Even for an internal package, there's no harm in saying in the README "This package provides the following internal functionality in the context of this other user-facing package. Please see the documentation of the main project."

Can you add [noblock] to your comments?

I would, but I'm actually not sure that this registration should go through in its current state. If there was enough context to understand the justification for the name, and with a minimal README to explain what this does and that more complete documentation is forthcoming, sure.

Well, you still have three days to fill in a bit more context and for me to unblock, or otherwise, maybe this needs a bit more community discussion on Slack.

Edit: [noblock]

@wsmoses
Copy link

wsmoses commented May 15, 2024

[noblock]

Yeah it is destined for end user use. In particular, copying a brief description I made in EnzymeAD/Enzyme.jl#805 (comment)

"""
Scheduling. Like I said at the top this is key to good performance, and incidentally distinct from AD. I have started playing with a repo Reactant.jl (https://github.com/EnzymeAD/Reactant.jl) which aims to resolve this. It can take a julia function and compile it into MLIR and run fancy optimizations on top of it, including using EnzymeMLIR for AD, and create relevant executables for CPU/GPU/TPU via XLA. It is very much in progress, but nevertheless a problem outside Enzyme now.
"""

And oh, my apologies I didn't realize that registration required docs. This is my first time registering a non-jll package, and that process seemed pretty cookie cutter.

In any cases I added a (reasonably cautious) README here, if that looks good to you @goerz : https://github.com/EnzymeAD/Reactant.jl

Part of my hesitation about end user use at the moment (which I try to caution above), is the method we use for compilation is very subject to change and may not end up following semantic versioning (e.g. the API will try to follow, but what input programs are supported with that same API is presently subject to change as we discover limitations from the underlying infrastructure). This is admittedly true of all compilers (including Julia within minor versions, in spite of best effort), but I anticipate the potential swing here to be rather wide to start with. Given a lot of folks use Enzyme and can reasonably assume stability of what is supported, I didn't want to give anyone that impression here -- at least to start with.

@wsmoses
Copy link

wsmoses commented May 15, 2024

and yes the name choice is explicit. Whereas Enzyme deals with differentiaiton. Reactant is an "execution engine". They are both separately useful for reasons specified in each doc in their own right, but they do work well together (and a reactant is an input to an Enzyme)

edit: [noblock]

@goerz
Copy link
Member

goerz commented May 15, 2024

[noblock] Looks great! All good!

@wsmoses
Copy link

wsmoses commented May 15, 2024

and yeah part of why I decided to register presently is that I learned people were starting to test it out in non-general registries, which seemed like a good reason to register it (in spite of my heavy warnings about potential semantic assurances).

[noblock]

@goerz
Copy link
Member

goerz commented May 15, 2024

[noblock] “Stable” is very much not a prerequisite for registration. Only “has some functionality” and “a general Julia user can have an idea of what the package is for and how to use it” (aka documentation / README). So this seems absolutely fine to put in General at this point.

may not end up following semantic versioning

You’re registering version 0.1 and the SemVer spec says “anything goes before 1.0”. So you can make the absolute wildest API changes in any subsequent 0.x.y release. Nobody should expect anything stable at all at this point. So, you’re totally fine (and perfectly following semantic versioning)

@goerz
Copy link
Member

goerz commented May 15, 2024

[noblock] (of course you still have to fix the compat issues and whatever is causing the package not to load)

UUID: 3c362404-f566-11ee-1572-e11a4b42c853
Repo: https://github.com/EnzymeAD/Reactant.jl.git
Tree: a24822bc0f4cdf6a1384f7e869885473bc410e09

Registrator tree SHA: 17aec322677d9b81cdd6b9b9236b09a3f1374c6a
@JuliaRegistrator JuliaRegistrator force-pushed the registrator-reactant-3c362404-v0.1.0-e32b514fa5 branch from b6afa40 to b0e918d Compare May 16, 2024 08:51
JuliaRegistrator referenced this pull request in EnzymeAD/Reactant.jl May 16, 2024
@JuliaTagBot JuliaTagBot merged commit dc1a66f into master May 18, 2024
11 checks passed
@JuliaTagBot JuliaTagBot deleted the registrator-reactant-3c362404-v0.1.0-e32b514fa5 branch May 18, 2024 03:10
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.

4 participants