opm-grid Let Entity return geometry as a copy.

Geometry is lightweight enough and this is what DUNE's grid interface mandates. Performance for norne stays the same. Closes PR #328

Jun 19 '18 10:06 blattms

jenkins run norne please

Jun 19 '18 10:06 blattms

I don't get why this should change any bit of the solution, will rerun Jenkins to be certain it was not a temporary fluke in the midst of a data update or something.

Jun 25 '18 09:06 atgeirr

jenkins build this please

Jun 25 '18 09:06 atgeirr

To be clear, I want to eventually merge this, but the failing checks makes me nervous and I want it to be investigated before merging.

Jun 28 '18 12:06 atgeirr

jenkins build this please

Aug 01 '18 11:08 atgeirr

The problem should still be there. And I have no idea what the problem really is.

Aug 01 '18 12:08 blattms

The problem should still be there. And I have no idea what the problem really is.

I retested because I could not reproduce locally. Do you get the test failures on your Linux system?

Aug 01 '18 13:08 atgeirr

Let me check again.

Aug 02 '18 09:08 blattms

Yes I do get

/home/mblatt/src/dune/opm-2.6/opm-upscaling/tests/common/test_gravitypressure.cpp(317): error: in "/BaseCase": difference{0.111111} between c[i]{-999.99999999999977} and e[i]{-899.99999999999977} exceeds 0.0001%
/home/mblatt/src/dune/opm-2.6/opm-upscaling/tests/common/test_gravitypressure.cpp(317): error: in "/BaseCase": difference{0.428571} between c[i]{-999.99999999999977} and e[i]{-699.99999999999977} exceeds 0.0001%
...

when running test_gravitypressure. But I am unsure whether this is the same as the error that we got last time.

Aug 02 '18 09:08 blattms

I have an upcoming fix for upscaling.

Aug 02 '18 10:08 blattms

jenkind build this with opm-upscaling=261 please

Aug 02 '18 10:08 blattms

jenkins build this opm-upscaling=261 please

Aug 02 '18 10:08 blattms

Beware: PR OPM/opm-upscaling#261 should be merged before this one.

Aug 02 '18 10:08 blattms

Thanks for fixing the first test error, it really did expose a bug in the old code. The second error (ignoring the TCPU thing that will be fixed soonish) is still not clear to me. Can you reproduce it? Do you get identical results apart from timing? The solution is not changing much, it is the fact that it changes at all that has me worried.

Aug 02 '18 11:08 atgeirr

Not sure what you mean

Aug 03 '18 08:08 blattms

Not sure what you mean

I think you have to be more specific, which sentence(s) is(are) unclear?

Aug 03 '18 08:08 atgeirr

Sorry for not being clear. I wondered what the second failure is that you are seeing. I only saw the TCPU stuff.

Aug 03 '18 12:08 blattms

There are two failures remaining, and only one is the TCPU one. In Jenkins only one is visible until you click the Test Result link, it's confusing. Direct link to failed test:

https://ci.opm-project.org/job/opm-grid-PR-builder/71/testReport/(root)/mpi/compareECLFiles_flow_SPE1CASE2_THERMAL/

Aug 03 '18 12:08 atgeirr

Seems like I messed up this one, too. Was magically closed and has no commits now.

Aug 03 '18 17:08 blattms

jenkins build this please

Aug 03 '18 17:08 blattms

Unfortunately, I am having troubles reproducing the memory errors in test_transmissibilitymultipliers

Aug 09 '18 11:08 blattms

/home/akva/kode/opm/super-build/opm-grid/opm/grid/cpgrid/../CpGrid.hpp: In member function ‘const Vector& Dune::CpGrid::cellCentroid(int) const’:
/home/akva/kode/opm/super-build/opm-grid/opm/grid/cpgrid/../CpGrid.hpp:969:97: warning: returning reference to temporary [-Wreturn-local-addr]
             return current_view_data_->geomVector<0>()[cpgrid::EntityRep<0>(cell, true)].center();

Aug 09 '18 11:08 akva2

==20424==    at 0x8A54EE: double Opm::UgGridHelpers::getCoordinate<Opm::UgGridHelpers::CpGridCentroidIterator<&(Dune::CpGrid::cellCentroid(int) const)> >(Opm::UgGridHelpers::CpGridCentroidIterator<&(Dune::CpGrid::cellCentroid(int) const)>, int) (GridHelpers.hpp:395)
==20424==    by 0x8A0B95: void tpfa_htrans_compute<Dune::CpGrid>(Dune::CpGrid const*, double const*, double*) (TransTpfa_impl.hpp:97)
==20424==    by 0x89B38C: void Opm::DerivedGeology::update<Opm::BlackoilPropsAdFromDeck, Dune::CpGrid>(Dune::CpGrid const&, Opm::BlackoilPropsAdFromDeck const&, Opm::EclipseState const&, double const*) (GeoProps.hpp:125)
==20424==    by 0x896871: Opm::DerivedGeology::DerivedGeology<Opm::BlackoilPropsAdFromDeck, Dune::CpGrid>(Dune::CpGrid const&, Opm::BlackoilPropsAdFromDeck const&, Opm::EclipseState const&, bool, double const*) (GeoProps.hpp:79)
==20424==    by 0x88AD48: TransmissibilityMultipliersCpGrid::test_method() (test_transmissibilitymultipliers.cpp:282)
==20424==    by 0x88AB3A: TransmissibilityMultipliersCpGrid_invoker() (test_transmissibilitymultipliers.cpp:257)
==20424==    by 0x8B6987: boost::unit_test::ut_detail::unused boost::unit_test::ut_detail::invoker<boost::unit_test::ut_detail::unused>::invoke<void (*)()>(void (*&)()) (callback.hpp:56)
==20424==    by 0x8B679C: boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke() (callback.hpp:89)
==20424==    by 0x6215CB0: ??? (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==20424==    by 0x61F5995: boost::execution_monitor::catch_signals(boost::unit_test::callback0<int> const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==20424==    by 0x61F61B2: boost::execution_monitor::execute(boost::unit_test::callback0<int> const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)
==20424==    by 0x6215DE1: boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::unit_test::test_case const&) (in /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0)

Aug 09 '18 11:08 akva2

Thread 1 "test_transmissi" received signal SIGSEGV, Segmentation fault.
0x00000000008a54ee in Opm::UgGridHelpers::getCoordinate<Opm::UgGridHelpers::CpGridCentroidIterator<&(Dune::CpGrid::cellCentroid(int) const)> > (t=..., i=0)
    at /home/akva/kode/opm/super-build/opm-grid/opm/grid/GridHelpers.hpp:395
395         return (*t)[i];
(gdb) where
#0  0x00000000008a54ee in Opm::UgGridHelpers::getCoordinate<Opm::UgGridHelpers::CpGridCentroidIterator<&(Dune::CpGrid::cellCentroid(int) const)> > (t=..., i=0)
    at /home/akva/kode/opm/super-build/opm-grid/opm/grid/GridHelpers.hpp:395
#1  0x00000000008a0b96 in tpfa_htrans_compute<Dune::CpGrid> (G=0x153fc20, 
    perm=0x154f070, htrans=0x154f670)
    at /home/akva/kode/opm/super-build/opm-grid/opm/grid/transmissibility/TransTpfa_impl.hpp:97
#2  0x000000000089b38d in Opm::DerivedGeology::update<Opm::BlackoilPropsAdFromDeck, Dune::CpGrid> (this=0x7fffffffab00, grid=..., props=..., eclState=..., grav=0x0)
    at /home/akva/kode/opm/super-build/opm-simulators/opm/autodiff/GeoProps.hpp:125
#3  0x0000000000896872 in Opm::DerivedGeology::DerivedGeology<Opm::BlackoilPropsAdFromDeck, Dune::CpGrid> (this=0x7fffffffab00, grid=..., props=..., eclState=..., 
    use_local_perm=false, grav=0x0)
    at /home/akva/kode/opm/super-build/opm-simulators/opm/autodiff/GeoProps.hpp:79
#4  0x000000000088ad49 in TransmissibilityMultipliersCpGrid::test_method (
    this=0x7fffffffd4b7)
    at /home/akva/kode/opm/super-build/opm-simulators/tests/test_transmissibilitymultipliers.cpp:282
#5  0x000000000088ab3b in TransmissibilityMultipliersCpGrid_invoker ()
    at /home/akva/kode/opm/super-build/opm-simulators/tests/test_transmissibilitymultipliers.cpp:257
#6  0x00000000008b6988 in boost::unit_test::ut_detail::invoker<boost::unit_test::ut_detail::unused>::invoke<void (*)()> (this=0x7fffffffd517, 
    f=@0x10ef518: 0x88ab18 <TransmissibilityMultipliersCpGrid_invoker()>)
    at /usr/include/boost/test/utils/callback.hpp:56
#7  0x00000000008b679d in boost::unit_test::ut_detail::callback0_impl_t<boost::unit_test::ut_detail::unused, void (*)()>::invoke (this=0x10ef510)
    at /usr/include/boost/test/utils/callback.hpp:89
#8  0x00007ffff6825cb1 in ?? ()
   from /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0
#9  0x00007ffff6805996 in boost::execution_monitor::catch_signals(boost::unit_test::callback0<int> const&) ()
   from /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0
#10 0x00007ffff68061b3 in boost::execution_monitor::execute(boost::unit_test::callback0<int> const&) () from /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0
#11 0x00007ffff6825de2 in boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::unit_test::test_case const&) ()
   from /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0
#12 0x00007ffff680d09e in boost::unit_test::framework_impl::visit(boost::unit_test::test_case const&) () from /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0
#13 0x00007ffff68434cb in boost::unit_test::traverse_test_tree(boost::unit_test::test_suite const&, boost::unit_test::test_tree_visitor&) ()
   from /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0
#14 0x00007ffff68089f6 in boost::unit_test::framework::run(unsigned long, bool) ()
   from /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0
#15 0x00007ffff6824287 in boost::unit_test::unit_test_main(bool (*)(), int, char**) ()
   from /usr/lib/x86_64-linux-gnu/libboost_unit_test_framework.so.1.58.0
#16 0x000000000088a227 in main (argc=1, argv=0x7fffffffe348)
    at /usr/include/boost/test/unit_test.hpp:59

Aug 09 '18 13:08 akva2

Just a small note: I did look into this more, but it seems like this is cannot be fixed easily as some of the code in GridHelpers relies on references that can be turned into pointers (e.g. for the cell centers).

Aug 23 '18 12:08 blattms

This could actually work now since the failing code is no more. Will rebase and retry.

Oct 17 '19 14:10 blattms

jenkins build this please

Oct 17 '19 14:10 blattms

This will not make it to the release as we have to touch a lot of code relying on this and that is bound to break easily.

Oct 30 '19 13:10 blattms