search

switch-model / switch

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

Switch 2.0.6 #115

Closed mfripp closed 2 years ago

mfripp commented 5 years ago

(Note: most of this comment and Josiah's response have been moved to issue #129 for future implementation.)

This is a place to discuss the next release of Switch. These are my immediate goals:

mfripp commented 5 years ago

@rodrigomha and @josiahjohnston I added a commit with simplified code for the rhosetter in the PySP example, with no dependency on sympy. I think my code should work, but it seems like the underlying implementation was probably not working already. e.g., when I run the command below, I get a message that it has data for gen_max_capacity_factor, but the key for the data is not in the indexing set VARIABLE_GEN_TPS_RAW.

Since this is your code, would you be able to see what you can do to get it to work?

(I'm also thinking we should have a bigger test set that we run before each release to make sure all our examples (and some bigger ones) run and produce the same results as the previous release.)

Here's the command I used that gave an error:

python -m pdb `which runph` --model-directory=. --instance-directory=inputs/pysp_inputs --solver=gurobi --default-rho=1000.0 --traceback --rho-cfgfile=rhosetter.py  --solution-writer=pyomo.pysp.plugins.csvsolutionwriter --output-scenario-tree-solution

Here were the pdb commands I used to diagnose the error (a little): 

l 861, 892
!key, val
!self._setitem_when_not_present(self._validate_index(key), val)
!self._validate_index(key)
!self.name
!list(self.keys())
!self.get_index_set().name
josiahjohnston commented 5 years ago

Dependencies

Retiring the inefficient & roundabout sympy parsing in favor of direct Pyomo calls seems great.

Dropping the direct numpy dependency in the advanced section won't make a difference because pandas is a core dependency and it has a dependency on numpy.

Consolidating the rest of the optional dependencies into a single section called [extras] seems fine to me.

I'm not a fan of making users do ad-hoc installations, and I don't understand benefits of doing that vs registering them explicitly as optional bits in extras_require.

My preference is to keep testfixtures in the core requirements since the testing framework is an integral part of software development, and at this point Switch still seems best-suited for people can do some coding. Hopefully Switch will grow into a broader audience, but in the meanwhile, it seems important to make it easy for people to start developing Switch to expand the base of advanced users & contributors. Moving testfixtures into [extras] would be acceptable to me, although I don't understand the problem in having it as a core dependency.

Maybe you can elaborate on the motivation for reducing dependencies? Design esthetics or some practical problems?

Installations

The instructions for casual users seems fine to me. For advanced users who will be working with code, I strongly recommend using some kind of virtual environment. Conda's environments seem fine as far as I know, as is virtualenv. I started mucking with pipenv (which seems to be taking over from virtualenv), but I haven't gotten far enough with pipenv to recommend it or write instructions.

I always recommend using pip install path/to/src over python path/to/src/setup.py install. Pip has superseded direct execution of setup.py for several years. See StackOverflow for a summary of the benefits.

I've played with the idea of setting up a docker image for Switch, but didn't know of a concrete use case. If that ever seems of interest to anyone, I could do that pretty easily.

Source code access

I like your idea of switch find <module>; its implementation should be trivial and it will lower the learning curve for people who aren't yet python gurus.

I'm not sure how or if the switch install examples is better than having users download the examples directory and save it where ever they want. I don't think it would be a big help to casual users who prefer graphical interfaces to command line interfaces, and advanced users who don't mind command line interfaces can do a cp directly for greater transparency. Would that command also download the expamples from a corresponding version of the git repo?

Adding something like --trace [[<module>|<file>[:<function>|<line>]] argument to switch solve seems like a nice idea. I know pdb & ipdb have internal commands for setting arbitrary breakpoints in other files, but I don't know how to interface with them programatically. Ideally, our code would use the most advanced debugger library available in the running python environment (see code snippet below from a different project doing related stuff). Personally, I find the tab-completion & color highlighting in ipdb incredibly useful compared to pdb.

def get_debugger_func():
    """Get a function that will drop the code into an interactive python
    session for debugging and development. Use the most sophisticated
    debugging library that is available (IPython > ipdb > pdb).
    N.B. these will not work in the context of parallel processing.
    """
    try:
        from IPython import embed
        return embed
    except ImportError:
        from ipdb import set_trace
        return set_trace
    except ImportError:
        from pdb import set_trace
        return set_trace

def main():
    # ...
    debug_func = get_debugger_func()
    debug_func()

rhosetter & Progressive Hedging

The problem was that the input .dat files needed to be regenerated since VARIABLE_GEN_TPS_RAW was included in the model. After dealing with that, there were a few bugs in your refactored code from wrong variable names. I fixed them and pushed the updates. It runs to completion now. The expected cost of the objective function doesn't match the value in the README (135,349,802 now vs 111,517,401 in the README), and I don't know if that's due to the solver I used, bug fixes since the last time the README was updated, or some undiagnosed bug. @bmaluenda, would you mind taking a look?

bmaluenda commented 5 years ago

I reviewed the PySP example and fixed several bugs, as well as updated the output files to match the format of PySP's current version.

I too am getting objective function values of about 135 MMUSD. I checked the output files and manually estimated the value of the OF and got closer to this value than to the previous ~110. I suspect that we hadn't re-generated the input files and re-run the example since I first uploaded it, so any changes to Switch's OF hadn't been reflected in the output files of this example.

I vote to keep these updated output files and consider them to be correct.

josiahjohnston commented 5 years ago

Thanks Benjamin!

josiahjohnston commented 5 years ago

Just pushed some more updates that pass all tests in both python 2 & 3:

josiahjohnston commented 5 years ago

FYI, I pushed the rebased version of this branch, which I had previously been calling 2.0.6-dev. Leaving it named next_release means we don't have to recreate this pull request, and will be less confusing if we decide to make the next release 2.1.

I updated the outstanding pull requests to be simple differences from the current tip of this branch, and closed an 2-year stale pull request (#97) by dividing into a commit of coding style that is mathematically equivalent, and an issue (#123) for discussing how the formulation is confusing and possibly buggy.

mfripp commented 2 years ago

Closing this because it was actually the 2.0.6 branch, not the next_release branch.