search

NREL / GEOPHIRES-X

MIT License
26 stars 21 forks source link

No longer allowing user to set output units #162

Closed malcolm-dsider closed 3 weeks ago

malcolm-dsider commented 3 months ago

Parameter processing has changed, and it no longer allows the user to set the output format. These lines added to the input file used to allow the user to set the units on the output display:

Output unit conversions you wish to make

Units:Bottom-hole temperature, degF, ---[This is what I want the units to be for this output parameter Units:Exploration cost,MEUR, ---[This is what I want the units to be for this output parameter Units:O&M Make-up Water costs, MEUR/yr

Part of the problem here might be that the currency conversion is currently broken (the server is offline), so the last two lines are likely to fail because of that, but the first line should work. It used to, and doesn't any longer.

malcolm-dsider commented 3 months ago

I have fixed the bug with respect to changing output units. Waiting for code review to do pull request so the code moves to production. Did NOT address the issue where the output units need to be changed but includes a currency conversion. This is a larger problem related to the fact that our currency conversion library no longer works - the web service it used to get near real-time currency conversion values has been shutdown, it seems. We need to find another, if possible, otherwise, remove the functionality.

softwareengineerprogrammer commented 3 months ago

@malcolm-dsider best way to start code review is to file a PR. If you don't feel confident about merging it into main repo yet, you can do a PR into your own fork by choosing the base as your fork; example PR I've done for my own fork: https://github.com/softwareengineerprogrammer/GEOPHIRES-X/pull/16 (and list of all PRs into my own fork I've done: https://github.com/softwareengineerprogrammer/GEOPHIRES-X/pulls?q=is%3Apr+is%3Aclosed)

softwareengineerprogrammer commented 3 months ago

Minor note: this might be partially/orthogonally related to https://github.com/NREL/GEOPHIRES-X/issues/95?title=Fix+missing+%+unit+for+SUTRA+pump+efficiency

malcolm-dsider commented 3 months ago

@malcolm-dsider best way to start code review is to file a PR. If you don't feel confident about merging it into main repo yet, you can do a PR into your own fork by choosing the base as your fork; example PR I've done for my own fork: softwareengineerprogrammer#16 (and list of all PRs into my own fork I've done: https://github.com/softwareengineerprogrammer/GEOPHIRES-X/pulls?q=is%3Apr+is%3Aclosed)

It is unclear to me how to do a PR into my own fork - I tried, but when I do, it says "there is nothing to compare." This, again, shows my ignorance of how GitHub works... perhaps we could do a Google Meet to show me how to do the PR, then do the code review?

softwareengineerprogrammer commented 3 months ago

@malcolm-dsider A PR into the main repo is OK if you're getting tripped up on a PR into your own fork (it's slightly less logistically preferable in the abstract in this case but also acceptable and better than spending inordinate time on fork PR)

softwareengineerprogrammer commented 3 weeks ago

Output units in general are working but currency conversion is not working - see https://github.com/NREL/GEOPHIRES-X/issues/236?title=Currency+conversions+not+working