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

Fixed missing unit conversion for total fluid in place #987

Merged
merged 2 commits into from
Dec 16, 2016
Merged

Fixed missing unit conversion for total fluid in place #987

merged 2 commits into from
Dec 16, 2016

Conversation

babrodtk
Copy link
Contributor

This (hopefully) fixes the issue with SPE1CASE2, in which the field total fluid in place is reported in the wrong units.

@babrodtk
Copy link
Contributor Author

@atgeirr can you take a look at this and see if it solves the reported issue on your end?

fip[i][6] = unit::convert::to(fip[i][6], unit::barsa);
}
else if (units.getType() == UnitSystem::UnitType::UNIT_TYPE_METRIC) {
fip[6] = unit::convert::to(fip[6], unit::barsa);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To guard agains future trouble, we should add a final else branch which throws (in case someone tries to run with LAB units).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Just pushed an update with that

@atgeirr
Copy link
Member

atgeirr commented Dec 16, 2016

It solves the reported issue, thank you for the fix!

Will merge when green.

@atgeirr atgeirr merged commit 7c88fad into OPM:master Dec 16, 2016
@totto82
Copy link
Member

totto82 commented Dec 16, 2016

Do we need the same fix for flow_ebos?

@atgeirr
Copy link
Member

atgeirr commented Dec 16, 2016

Do we need the same fix for flow_ebos?

Unfortunately, yes, since it duplicates rather than reuse the simulator class.

@andlaus
Copy link
Contributor

andlaus commented Dec 16, 2016

Unfortunately, yes, since it duplicates rather than reuse the simulator class.

IIRC, the reason why flow_ebos does this is that the current FIP-code sometimes uses ADBs and other stuff that depends on Eigen which is not available in flow_ebos. (as far as I can see, the functionality of ADBs and Eigen's vectors are not really required, though.) we probably need to clean that code up a bit...

hnil pushed a commit to hnil/opm-simulators that referenced this pull request Jan 31, 2018
Refactor relperm diagnostics using OpmLog functionalities.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants