-
Notifications
You must be signed in to change notification settings - Fork 26
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
Fix/enable Wanju_Yuan_Closed-Loop_Geothermal_Energy_Recovery.txt unit test case #16
Comments
@malcolm-dsider Should we just delete this case, or can/should it be fixed? Talked to a user yesterday who was trying to run it and getting hung up on it not workign |
Whatever the problem is, it needs to be fixed. The code from Wanju enables our multi-lateral closed-loop functionality. The example reproduces values from a published paper. The example works on my instance. Is the problem in the differences in these values: Surface power plant costs: 6.74 vs. 6.12; CAPEX: 36.79 vs. 36.18; and Total surface equipment costs: 7.24 vs. 6.63? If so, then the problem is likely caused by the update in the cost metrics, not anything in the closed-loop code. If so, we just need to update the "right" values in the output so they match. If anyone is having trouble with closed-loop parts of the code, you should probably direct them to me since I wrote that section. |
@malcolm-dsider yep it's a diff of the values; apologies for the curveball re: the user issue, that was actually a red herring as it turns out.
It's those and a few others (you can ctrl+f for |
Yes
…________________________________
From: Jonathan Pezzino ***@***.***>
Sent: Thursday, November 30, 2023 4:41:42 PM
To: NREL/python-geophires-x ***@***.***>
Cc: Malcolm Ross ***@***.***>; Mention ***@***.***>
Subject: Re: [NREL/python-geophires-x] Fix/enable Wanju_Yuan_Closed-Loop_Geothermal_Energy_Recovery.txt unit test case (Issue #16)
@malcolm-dsider<https://github.com/malcolm-dsider> yep it's a diff of the values; apologies for the curveball re: the user issue, that was actually a red herring as it turns out.
Surface power plant costs: 6.74 vs. 6.12; CAPEX: 36.79 vs. 36.18; and Total surface equipment costs: 7.24 vs. 6.63?
It's those and a few others (you can ctrl+f for ^ through the block above I posted for the full list of discrepancies). Sounds like some of these deviations are expected and we can just update the test with the current output for that input?
—
Reply to this email directly, view it on GitHub<#16 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AWGVYT3IVEZZV6APSRJTFFDYHEDSNAVCNFSM6AAAAAA54VIRVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZUGY3TIOBUHE>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Addressed by softwareengineerprogrammer@950e939 & softwareengineerprogrammer@3c79eae which will be included in #66 |
Revenue & Cashflow Profile
If the filter in
test_geophires_examples
is updated to match/include Wanju_Yuan_Closed-Loop_Geothermal_Energy_Recovery.txt, the test fails with the following error/diff:The text was updated successfully, but these errors were encountered: