respect the fact that non-SI temperature may be offset from zero

[andlaus]

this is another attempt for fixing the "measure" temperature conversion stuff after #1203 without causing a trainwreck.

I did not find any substantial mistakes in #1203, so maybe some downstream code tries to work around the missing offset and thus creates a wrong result (then the downstream code needs to be fixed), maybe the reference data needs to be updated, or both. Anyway, temperature conversion is now much more thoroughly tested...

[andlaus]

jenkins build this with downstreams please

[joakim-hove]

so maybe some downstream code tries to work around the missing offset and thus creates a wrong result

That is my hunch.

[joakim-hove]

jenkins build this update_data please

[joakim-hove]

OK - this looks fine; and modulo update_data I am nearly ready to merge it. But to be honest I fear there might be hardcoded 273.xx around - maybe in opm-output? So - would appreciate a search for such obscenities first.

[andlaus]

okay, I've updated the PR for the first review comment and looked at the test failures in opm-simulators: all are caused by legacy simulators. This is reasonable because without OPM/opm-simulators#1391 (?) temperature is not written to ECL restart files by the non-legacy simulators. @totto82: can you please test that temperature output says correct with OPM/opm-simulators#1391 if the present PR gets merged.

the new temperatures written by the legacy simulators is both more correct than the current master but it is still wrong: it is more correct because with the present PR, °F instead of °R are used but they are wrong because the legacy simulators seem to hardcode a reservoir temperature of 20 °C instead of 60 °F (which is 15.666°C).

[andlaus]

jenkins build this with downstreams update_data please

[atgeirr]

hardcode a reservoir temperature of 20 °C instead of 60 °F

Is 60 degrees F the "eclipse default"? If so we should just change it and be done.

[andlaus]

Is 60 degrees F the "eclipse default"? If so we should just change it and be done.

it seems to be the E100 default, E300 defaults to 100°C as far as I can tell. (IMO the E300 default is much more sensible.)

On the OPM side, this havoc is basically caused by the fact that the legacy simulators do not use the fluid system to determine the reservoir temperature. Anyway, the legacy simulators do not really use the temperature, so the outputted values are basically "eye candy" and thus it's probably better not to change that code.

[jenkins4opm]

Existing PR https://github.com/OPM/opm-data/pull/303 was updated

[atgeirr]

Please don't merge this until @totto82's big output-PR is merged (expected tomorrow morning).

[andlaus]

the output PRs have been merged. let's see which reference solutions are affected.

[andlaus]

jenkins build this with downstreams update_data please

[jenkins4opm]

Existing PR https://github.com/OPM/opm-data/pull/303 was updated

[andlaus]

I looked at all the jenkins failures, and all of them are caused by the TEMP field going from absolute temperature (i.e., °R or K) to relative ones (°F or °C). I suppose that this can be merged if jenkins succeeds with the new reference results?

[andlaus]

jenkins build this with downstreams opm-data=303 please

[atgeirr]

I looked at all the jenkins failures, and all of them are caused by the TEMP field going from absolute temperature (i.e., °R or K) to relative ones (°F or °C). I suppose that this can be merged if jenkins succeeds with the new reference results?

So this is nice bugfix! Sounds good to me. I guess it also means that our fears of code further downstream manually applying an offset were false?

[andlaus]

arg I forgot to force push the fix from review! re-jenkins please!

[andlaus]

jenkins build this with downstreams opm-data=303 please

[andlaus]

I guess it also means that our fears of code further downstream manually applying an offset were false?

it looks like it. maybe there was just some confusion of whether the "first" or the "second" values are the current or the reference results?

[joakim-hove]

jenkins build this opm-data=303 please

[andlaus]

alright, Jenkins is green. @joakim-hove: mind merging?

[joakim-hove]

mind merging?

No - I do not mind merging!