Teuchos: Add operator bool to RCP class

[pyt-viper]

Enhancement

@trilinos/teuchos

Teuchos::RCP does not have an operator bool, but could benefit from one. This would make it behave more like std:shared_ptr. See #6607 for a justification.

Blocks #6607

[bartlettroscoe]

This type of operator needs to be added very carefully or it can create a mess with the type system. I know there are special methods to implement this type of method safely for its intended usage but it would need to be done carefully to match how std::shared_ptr does it. Otherwise, I support adding more functions to Teuchos::RCP to mirror std::shared_ptr so the two can be used interchangeably (for the most part).

[pyt-viper]

@bartlettroscoe Looking at the std::shared_ptr code, which is pretty short, it looks like the important thing is the explicit keyword, to prevent implicit conversions. Locally, I've gotten this to pass all of the Teuchos tests, and I am testing downstream packages now.

[bartlettroscoe]

Looking at the std::shared_ptr code, which is pretty short, it looks like the important thing is the explicit keyword, to prevent implicit conversions

Okay, it looks like C++11 has saved the day here by allowing conversion functions to be marked explicit:

• https://www.kdab.com/explicit-operator-bool/

This would be a great PR to add. Thanks!

[pyt-viper]

Testing downstream packages has produced some failures, and I am now checking to see if they involve RCPs. They first couple I have looked at have not. I’ll keep checking as time permits.

[bartlettroscoe]

Testing downstream packages has produced some failures, and I am now checking to see if they involve RCPs. They first couple I have looked at have not. I’ll keep checking as time permits.

That is troubling. I would have to see the details of that. Can you post a WIP PR so I can inspect this?

[pyt-viper]

I will push my changes and make that PR shortly. Right now, I am in the middle of running the same tests (enabling Teuchos and all forward dependent packages) with my changes commented out. So far, the I am seeing the same failures, so my changes can't be responsible.

It's probably a pretty non-standard build. I had to disable Xpetra, STK, and PyTrilinos, as well as ML tests, ROL examples, and TrilinosCoupling examples in order to get it to build. It produces 2283 tests.

Here is my `cmake` invocation:

#! /bin/sh

EXTRA_ARGS=$@
BASE=`conda info | grep "active env location" | cut -d: -f2 | sed -e 's/^[[:space:]]*//'`
MPI_BASE=$BASE

rm -f CMakeCache.txt
cmake \
  -D CMAKE_BUILD_TYPE:STRING=DEBUG \
  -D CMAKE_INSTALL_PREFIX:PATH=install \
  -D BUILD_SHARED_LIBS:BOOL=OFF \
  -D TPL_ENABLE_MPI:BOOL=ON \
  -D MPI_BASE_DIR:PATH=$MPI_BASE \
  -D MPI_EXEC:FILEPATH=$MPI_BASE/bin/mpiexec \
  -D MPI_EXEC_PRE_NUMPROCS_FLAGS:STRING="--mca;oob_tcp_if_include;lo0" \
  -D TPL_ENABLE_BoostLib:BOOL=ON \
  -D TPL_ENABLE_HDF5:BOOL=ON \
  -D Netcdf_LIBRARY_DIRS=$BASE/lib \
  -D Netcdf_INCLUDE_DIRS=$BASE/include \
  -D Matio_LIBRARY_DIRS=$BASE/lib \
  -D X11_LIBRARY_DIRS=$BASE/lib \
  -D Trilinos_ENABLE_EXPORT_MAKEFILES:BOOL=OFF \
  -D Trilinos_ENABLE_ALL_FORWARD_DEP_PACKAGES:BOOL=ON \
  -D Trilinos_ENABLE_Fortran:BOOL=OFF \
  -D Trilinos_ENABLE_TESTS:BOOL=ON \
  -D Trilinos_ENABLE_EXAMPLES:BOOL=ON \
  -D Trilinos_ENABLE_Teuchos:BOOL=ON \
  -D Trilinos_ENABLE_Thyra:BOOL=ON \
  -D Trilinos_ENABLE_Xpetra:BOOL=OFF \
  -D Trilinos_ENABLE_STK:BOOL=OFF \
  -D Trilinos_ENABLE_PyTrilinos:BOOL=OFF \
  -D ML_ENABLE_TESTS:BOOL=OFF \
  -D ROL_ENABLE_EXAMPLES:BOOL=OFF \
  -D TrilinosCouplings_ENABLE_EXAMPLES:BOOL=OFF \
  $EXTRA_ARGS \
  ..

I use Anaconda to provide TPL support, which explains my BASE definition.

Here are the errors I am seeing:

1156 - Belos_CustomSolverFactory_MPI_4                      No such file or directory (errno 2)
1296 - Ifpack2_BlockTriDiContainerUnitAndPerfTests_MPI_4    Memory leak
1300 - Ifpack2_ContainerFactory_MPI_1                       Memory leak
1304 - Ifpack2_TomVankaTest_MPI_4                           Memory leak
1351 - FEI_elemDOF_Aztec_MPI_2                              Segmentation fault
1359 - FEI_lagrange_20quad_old_MPI_2                        Segmentation fault
1360 - FEI_lagrange_20quad_old_MPI_4                        Segmentation fault
1368 - FEI_multifield_vbr_az_MPI_2                          Segmentation fault
1369 - FEI_multifield_vbr_az_MPI_3                          Segmentation fault
1388 - Teko_testdriver_MPI_1                                Strategy "The Cat" could not be constructed
1895 - Rythmos_StepperBuilder_UnitTest_MPI_1                Test that code {builder = Teuchos::null;} does not throw : passed
2067 - Stokhos_SacadoTraitsUnitTest_MPI_1                   uncaught exception of type std::logic_error: Throw test that evaluated to true: 0==rcp_node_list()
2069 - Stokhos_SacadoPCEUnitTest_MPI_1                      ""
2070 - Stokhos_SacadoETPCEUnitTest_MPI_1                    ""
2071 - Stokhos_SacadoPCESerializationTests_MPI_1            ""
2072 - Stokhos_SacadoPCECommTests_MPI_1                     ""
2108 - Stokhos_sacado_example_MPI_1                         ""
2201 - ROL_test_vector_StdArrayInterface_MPI_1              Memory leak
2283 - PikeBlackBox_rxn_MPI_1                               Not reproduced when I ran it independently

Note: all of the failures were reproduced with my changes commented out (plus a couple of new timeouts...)

[mhoemmen]

@pyt-viper What's with the MPI_EXEC_PRE_NUMPROCS_FLAGS:STRING="--mca;oob_tcp_if_include;lo0" flag?

[bartlettroscoe]

@pyt-viper, the Trilinos PR tester will run some standard builds and if those pass, we should be in pretty good shape.

[pyt-viper]

@bartlettroscoe do you need to review the PR to set it in motion?

[pyt-viper]

@pyt-viper What's with the MPI_EXEC_PRE_NUMPROCS_FLAGS:STRING="--mca;oob_tcp_if_include;lo0" flag?

@mhoemmen that is a relic from an mpiexec problem from long ago. It might be obsolete.

[bartlettroscoe]

@bartlettroscoe do you need to review the PR to set it in motion?

@pyt-viper, if you are not a member of the Trilinos GitHub project, then yes, I will need to approve, I think to trigger a PR testing build. See:

• https://github.com/trilinos/Trilinos/wiki/Pull-Request-Testing-Interface

Also, the the merge base branch needs to be the 'develop' branch, not the 'master' branch'.

[pyt-viper]

Yeah, it's all ready to go, #6641

[pyt-viper]

Addressed by PR #6641