-
Notifications
You must be signed in to change notification settings - Fork 46
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
Green steel cleanup #153
Green steel cleanup #153
Conversation
Bugfix: Fixing tests and updating CapEx return of the Singlitico PEM cost model
…geration energy costs
* add rotated strands test for flicker * add flicker calc with MultiPoint
Distributed pressure vessel computations
…' into feature/offshore-h2
Feature/offshore h2
reconciliation between ckiefer and cfrontin changes
Feature/offshore h2
Green steel cleanup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general this looks good and I was able to replicate results for representable cases for green steel by running project code in the GreenSteel repository. However, there were a few very small things that I had to change within this HOPP branch to get those to work. I am unable to make comments directly to the code because the differential is too large (too many commits), so I am listing out the changes that I had to make below:
requirements.txt: I was unable to do pip install on this on a Windows 10 machine; something about accumulation-tree. I was, however, able to get things running by doing pip install on the feature/green_steel_ammonia branch requirements.txt file and then installing additional packages as necessary. It would be good to make sure that the intsallation instructions work for Windows machines though.
hopp/to_organize_hopp_tools_steel.py: I had to remove "project_path" from the inputs to hopp_for_h2 at line 646.
hopp/to_organize/H2_Analysis/hopp_for_h2.py: I had to comment out the code at line 169 to get wind+solar cases to run when using PySAM. This was not an issue when using Floris. I should note that this code was not in the feature/green_steel_ammonia branch. If we are going to keep it, we need to figure out how to get wind+solar cases to work while this piece of code is here.
hopp/to_organize/H2_Analysis/2020ATB_NREL_Reference_7MW_200.csv: I had to add this file from the feature/green_steel_ammonia branch to get things to run.
hopp/to_organize/distributed_pipe_cost_analysis.py: I had to add this file from feature/green_steel_ammonia branch to get distributed cases to run. I also had to change the pipe_info_dir code at line 16 to get things to run properly.
I approve the PR with these changes addressed. If helpful I can push these changes from my local repo to this branch to facilitate (some of) these changes.
Added and changed some files to get green steel code to work
* Remove redundant resource files * Fix clustering tests * Remove unneeded tests We determined in our last Refactor meeting that these are not real tests and should be removed. * Limit shapely version There's currently a range of shapely versions that work for HOPP, and this is reflected in `master`. If we want to loosen this constraint, we'll need to figure out why the tests fail on current versions of the package. * Fix test_csp imports * Revert PySAM to 3.0.0 This change broke tests in tests/hopp/test_hybrid, I'm not sure why, but we'll need to revisit if we want to upgrade. * Fix documentation build * Disable test_pressure_vessel * Fix test_custom_financial * Reorganize tests into tests/hopp * Disable analysis tests Met with Kaitlin Brunik (@kbrunik) today to discuss these particular analysis tests. These are based on outdated examples, and need to be re-assessed alongside the code in the holdover `to_organize` directory, which contains a number of files that likely belong as project code rather than usage examples for the repository. * Update Custom Financial Model (#195) * pull in changes from pysam_update_capacity * fix import * update tests * undo a commenting * BatteryStateless (#196) * pull in changes from pysam_update_capacity * fix import * update tests * add batttery_stateless * break out create_max_gross_profit_objective by tech * fix tests * fix minor comments * update .gitignore * Remove more resource files Another attempt at removing files that are not explicitly used in the code. * Re-add clobbered changes and fix imports --------- Co-authored-by: Darice L Guittet <[email protected]> Co-authored-by: Jared Thomas <[email protected]>
@jaredthomas68 Tests are now passing 👍 Though I realized I'm going to need to make a followup that re-enables Update: created #200 |
We are still working to get the tests all passing with this branch, but I think it is where you can start looking through it and trying it out with the GreenSteel project repository. There is also a "to_organize" directory that we will sort out over time. For now, if you are missing something that was in
examples
, look inhope/to-organize
Please direct discussion here so we can stay synced.