Skip to content
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

No longer allowing user to set output units #162

Closed
malcolm-dsider opened this issue Mar 18, 2024 · 6 comments
Closed

No longer allowing user to set output units #162

malcolm-dsider opened this issue Mar 18, 2024 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@malcolm-dsider
Copy link
Collaborator

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.

@softwareengineerprogrammer softwareengineerprogrammer added the bug Something isn't working label Mar 18, 2024
@malcolm-dsider
Copy link
Collaborator Author

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
Copy link
Collaborator

@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)

@softwareengineerprogrammer
Copy link
Collaborator

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
Copy link
Collaborator Author

@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
Copy link
Collaborator

@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
Copy link
Collaborator

Output units in general are working but currency conversion is not working - see #236

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants