-
Notifications
You must be signed in to change notification settings - Fork 546
Add a default rule for custom blocks #3570
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
base: main
Are you sure you want to change the base?
Conversation
We have been debating this on the developer's call. I think we are leaning toward a slightly different (and a little more general) implementation: Users want to (easily) create custom block classes (for that matter, we do, too). The catch is that they find themselves needing / wanting to pass in additional parameters / options to the class. IDAES does this by adding a This PR currently codifies the Alternately, we could declare the options in the decorator and pass them as keyword arguments to the rule. This would allow for, e.g.: @declare_custom_block("FooBlock", rule="build", rule_args=["capex", "opex"])
class FooBlockData(BlockData):
def build(self, *args, *, capex, opex):
self.x = Var(list(args))
self.y = Var()
self.capex = capex
self.opex = opex
model.foo = FooBlock(m.I, capex=42, opex=21) Another option is that we could just save all unexpected keyword arguments passed to the class @declare_custom_block("FooBlock", rule="build")
class FooBlockData(BlockData):
def build(self, *args, *, capex, opex):
self.x = Var(list(args))
self.y = Var()
self.capex = capex
self.opex = opex
model.foo = FooBlock(m.I, capex=42, opex=21) This would defer some error checking (you won't get the unexpected keyword argument error when you declare the component -- instead it would be when you construct it). BUT, this would make this change also work with all the existing components (not just blocks declared by Thoughts / comments welcome. I believe that the consensus from the most recent dev call was to lean toward the second alternative proposal. |
Thank you @jsiirola for the feedback! I agree that either of the suggested alternatives makes the code cleaner. If I understand things correctly, the first alternative is fairly easy to implement, and does not require too many changes. But the second alternative requires some major changes, and I'm not fully familiar with the code base to make the necessary changes. I agree that it is more general, but I have the following questions:
If the answer to both the questions is No, then I too agree that the second alternative would be much better. In that case, I will close this PR and try to implement the second approach. However, if the first alternative is sufficient, then I will make the necessary changes and push them to this PR. Please let me know your thoughts. Thank you! |
Fixes # .
Summary/Motivation:
Add a default rule for custom blocks. With this default rule, construction of custom blocks becomes easier.
Implementing
build
method is optional. If it is not implemented, an empty block will be returned.Users can overwrite the default
build
method by passing therule
argument:m.blk = FooBlock([1, 2, 3], rule=my_custom_block_rule) # Ignores the build method
Changes proposed in this PR:
declare_custom_block
decoratorLegal Acknowledgement
By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution: