-
Notifications
You must be signed in to change notification settings - Fork 1
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
Requirements specification #7
Comments
My preferences are
|
@findmyway it looked like you had some ideas in #1 - if you're up for it, you could take a stab at implementing something and then we could discuss it. |
My preferences:
|
|
@findmyway I like your idea for for (4)
If these are "industrial-grade" environment writers, like people wrapping gym or something, then I agree, but we should be careful - if it's someone in environmental engineering who has a cool model of a water system, we should not accidentally make them think they need to implement
Make sure to weigh in on #6 :) Ok. Given that we agree on (2), I think this package should contain:
where Anyone want to take a crack at it? |
Given the importance that |
Maybe it's just me, but I am getting confused here.
I think we should be more clear about who this check tool is intended for. Are we agreed on the role of the environment writer here? I agree with @findmyway that environments should be written w/o knowledge of algorithms/solvers. I was under the impression the requirements check was for algorithm writers:
@zsunberg the functions you listed in your last comment don't take algorithms into account in any way? Don't we need some information from the algorithm writer to say whether requirements are met? Or are you describing some kind of interface for algorithm writers? EDIT: Sorry, I think I found the source of my confusion. Is |
@darsnack Thanks for bringing this up - I think it is useful to talk about. Yes, you are right - There are really three roles and 2 contexts that we should be thinking about for this package: Roles:
Contexts:
environment developers should certainly do this, but environment tinkerers who only want to start out with one algorithm should probably start out only implementing the optional functions that are needed to use the algorithm that they want to. I see
Requirements is a mechanism for these communication flows:
In other words, I don't think algorithm tinkerers will ever specify requirements and I don't think environment developers will ever look at them. They are mostly for communication from developers to tinkerers, whereas |
Yeah, I'm not sure exactly what the usage would look like. I was imagining that a friendly algorithm writer might write something like this: function learn(env::AbstractEnv)
# construct requirements list
# ...
if !requirements_met(list)
@warn("There were missing requirements for <algorithm name>")
show_requirements(list)
end
# run the algorithm
end and/or, they might write a function called
I'm not sure if we can do anything more than that because we haven't specified how an algorithm should look (and shouldn't) in this package. Maybe the requirements functionality should go in another package, e.g. RLEnvironmentLinter.jl? |
Okay that clears a lot up. Based on that description, I think we need args + types as part of requirements for the |
Sorry, I wasn't clear. I imagined the situation, where a student or researcher, unfamiliar with the RL ecosystem in julia, has a custom environment and wants to run some algorithms on that environment. The researcher figured out that I like the idea of
|
In #1, several people talked about a way for algorithm writers to specify requirements. This seems like a good idea, but designing it will be challenging. Here are a few important questions we need to answer:
clone
) or method signatures (i.e. with the argument types, e.g.clone(::YourCommonEnv)
?There are several sub-issues with this:
interact!(env, a)
, to check the requirement we would have to infer (or ask the user to supply) the type ofa
. It might be possible to doBase.return_types(env->eltype(actions(env)), Tuple{typeof(env)}))
or something.clone
orsetstate!
Base.show_method_candidates
to make a report like aMethodError
, or just wait until it is erroneously called?Please weigh in!
The text was updated successfully, but these errors were encountered: