switch-model / switch

A Modern Platform for Planning High-Renewable Power Systems
http://switch-model.org/
Other
129 stars 85 forks source link

Minor updates to sets for functionality, speed and API conformity #110

Closed josiahjohnston closed 5 years ago

josiahjohnston commented 5 years ago

Add GENS_BY_ENERGYSOURCE set for faster/easier indexing. Apply the "construction dictionary" pattern to the 'GENS*' sets which might be making testing slightly faster. Update the set initialization in dispatch.py to match Pyomo documentation of supported API (initialize argument instead of rule). rule works for now, but using published API should be more future proof.

mfripp commented 5 years ago

Forgot to mention:

I think if not d is considered more Pythonic than if len(d) == 0. I would argue that the intent is less clear to new users, but nevertheless it better fits Python conventions and the expectations of more experienced users, so I went with it. It may also be slightly faster to check "falsiness" of a dictionary than to compare its length to 0, but I doubt it makes much difference.

Also, we can generally do del m.component instead of delattr(m, 'component').

josiahjohnston commented 5 years ago

I like shorter statements that are more pythonic. Generalizing this might be nice, especially if it helps with readability & consistency. Not sure how broadly we'd want to deploy the general form, since the timing benefits seem negligible for a lot of sets, and using a custom solution has a cost of readability/cognitive load. Do you have a sense of which set initializations are bottlenecks in your runs? In the long term, it might be nice to package this sort of augmentation into Pyomo pull requests.

In other languages with strict enforcement of private variables & methods, leading underscores can indicate special/secret behavior. Python doesn't enforce private vars or methods, but treats leading underscores as a helpful suggestion that the method or variable in question is meant for internal use (not part of a stable public interface) ..encoded into PEP-8. When I see those underscores in libraries that I'm working with, I'll try to avoid code against them because their implementation or existence might change in the next release.

For practical purposes, dropping leading underscores from this method should be fine. A method defined inside of another method is another way of indicating internal use. An underscore on these dictionary seems a little better to me, since that dictionary is just a side-effect of our optimization. But it isn't that big of deal to me either way, especially since that variable only exists during initialization - probably just for the duration of one for loop.