-
Notifications
You must be signed in to change notification settings - Fork 27
GeoH2: Arps decline curve #454
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: develop
Are you sure you want to change the base?
Conversation
jmartin4nrel
left a comment
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.
Everything LOOKs right at first glance... however, when I add a couple lines to the example script to look at the hydrogen_out profile, it looks like this...

Not the curve we're looking for, but idk why. It would be good to add a couple lines on the example script (04_geo_h2) to show both this plot and the year-of-year decline curve and make sure they're what we expect
| initial_wellhead_flow: 4000 | ||
| gas_flow_density: 0.1 | ||
| ramp_up_time_months: 6 #months | ||
| percent_increase_during_rampup: 0.05 |
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.
Maybe a terminology discussion that should happen in a standup meeting: when I say something is a "percent", then 5 percent goes in as 5., it doesn't go in as 0.05 - I would call it a "fraction" in that case
| def arps_decline_curve_fit(self, t, qi, Di, b): | ||
| """Arps decline model. | ||
| Relevant literature: https://doi.org/10.1016/j.jngse.2021.103818 |
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.
May be coming in the final PR, but I would put in first author name and brief description of what they did, plus page/equation number of the equations we used and mention of tables/SI we used to make the fits.
Arps Decline Curve Modeling
This feature add is to make it so it's possible to use the Arps decline curves described in Tang et al. when simulating natural geologic hydrogen production.
Section 1: Type of Contribution
Section 2: Draft PR Checklist
TODO:
Type of Reviewer Feedback Requested (on Draft PR)
Structural feedback:
Implementation feedback:
Other feedback:
@jmartin4nrel it would be great to get your initial feedback on the method and implementation. I've added the
arps_decline_curve_fitmethod directly to theNaturalGeoH2PerformanceModelclass. Users are able to use the decline curve with predefined fits from data ("Permian", "Eagle_Ford", "Bakken"), select their own "Di" and "b", or not select anything and it will decline linearly.Section 3: General PR Checklist
docs/files are up-to-date, or added when necessaryCHANGELOG.mdhas been updated to describe the changes made in this PRSection 3: Related Issues
Section 4: Impacted Areas of the Software
Section 4.1: New Files
path/to/file.extensionmethod1: What and why something was changed in one sentence or less.Section 4.2: Modified Files
path/to/file.extensionmethod1: What and why something was changed in one sentence or less.Section 5: Additional Supporting Information
Section 6: Test Results, if applicable
Section 7 (Optional): New Model Checklist
docs/developer_guide/coding_guidelines.mdattrsclass to define theConfigto load in attributes for the modelBaseConfigorCostModelBaseConfiginitialize()method,setup()method,compute()methodCostModelBaseClasssupported_models.pycreate_financial_modelinh2integrate_model.pytest_all_examples.pydocs/user_guide/model_overview.mddocs/section<model_name>.mdis added to the_toc.yml