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

[RFC][Relay] Feature Manager #3236

Closed
MarisaKirisame opened this issue May 23, 2019 · 6 comments
Closed

[RFC][Relay] Feature Manager #3236

MarisaKirisame opened this issue May 23, 2019 · 6 comments

Comments

@MarisaKirisame
Copy link
Contributor

During the development of relay passes, a problem come up repeatedly: a pass often get input it does not expect, and as a result, it fail (either implicitly, taking exponential time or produce incorrect output, or explicitly, typically when a functor does not get match). typical example is interpreter/aot compiler need fuseops ran (more precisely, it need no Op outside of PrimitiveFunction) and need the expr having no sharing, fold scale axis/fuse ops not working on pattern matching (will fuse through both of them, while they do special case on if), partial evaluator assuming no recursion right now, dead code eliminator assuming no effect...
I think we should have a feature manager, which, given a module, or an expr, will return a set of feature (feature is an enum). pass then can check that the input is good, by using the feature manager to calculate the given input, then checking to see if there is anymore.
we can also bake this into the pass manager: each pass will have to specify three more argument: FeatureSet input_features, FeatureSet add_features, FeatureSet remove_features, that denote which feature can this pass handle, which feature it may add, and which feature it will remove. This make the passmanager more robust (we can know whether a sequence is safe to run, before actually running it, and we can use this to guide random shuffling of pass).
@tqchen @zhiics @jroesch @slyubomirsky any idea?

@slyubomirsky
Copy link
Contributor

I do like the idea of documenting what a pass can and cannot handle and ensuring that input (if possible) is appropriately sanitized or errors out immediately. There are a few analyses we should implement to facilitate writing passes anyway, like checking for effects, so having checks built into the pass manager would likely make a lot of people's lives simpler. (I don't know that we need add_features and remove_features: a pass could just list features it cannot handle and the manager could run analyses to search for those features in the pass input. Do you have examples in mind for when that wouldn't work?)

@MarisaKirisame
Copy link
Contributor Author

@slyubomirsky they give stronger gurantee: when the whole pipeline fail, we can point our finger at one pass that is at fault (some pass take tons of feature and just pass them further down the pipeline, so it isnt always the case that the previous pass is at fault).

@slyubomirsky
Copy link
Contributor

True

@zhiics
Copy link
Member

zhiics commented May 24, 2019

Yes, I agree this is annoying. It looks we might need to introduce some metadata for a pass. Usually when we do sequential passes, we may need to consider about preserving information from the updated passes and also validate if we can proceed. We should think about it more when we start resolving pass dependency and tackling phase ordering. How do you guys think?

@tqchen
Copy link
Member

tqchen commented May 24, 2019

I think this is something that could be implemented by Sequential. Instead of rolling a separate manager, let us just rolling in such features incrementally. For example, the pass could declare it can only take in ANF or graph form, and the Sequential can be equipped to either resolve automatically or insert checking pass

@MarisaKirisame
Copy link
Contributor Author

I call it feature manager because calling it feature sounds strange. My plan is to implement feature (an enum), feature set (a datastructure recording the enum), and functions on them. They will be used in the pass manager.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants