Remove all instances of array1d<string>

[rrsettgast]

In an attempt to identify some errors we are seeing on cuda systems, I removed all array1d<string> from the code and replaced it with string_array which aliases std::vector<string>....it remains to be seen if this fixes the problem.

The error can be seen here: https://github.com/GEOS-DEV/GEOS/actions/runs/7674651265/job/20919616872?pr=2873

[TotoGaz]

For info, what's the problem?

[wrtobin]

There was some motivation coming from an error on frontier, but IIRC we've since moved past that.

Regardless, there is never a reason (and should never be, by design) we would need to move strings between memory spaces, which is one of the primary purposes of the lvarray arrays. String array1ds have also been the most likely case to fail in lvarray unit tests when making changes to lvarray itself, so if we don't need to support that usage it will probably save a bit of dev time.

[corbett5]

There was some motivation coming from an error on frontier, but IIRC we've since moved past that.

Regardless, there is never a reason (and should never be, by design) we would need to move strings between memory spaces, which is one of the primary purposes of the lvarray arrays. String array1ds have also been the most likely case to fail in lvarray unit tests when making changes to lvarray itself, so if we don't need to support that usage it will probably save a bit of dev time.

On the other hand I can think of twp reasons for the status-quo. First is that everything is stored as an LvArray:Array and we don't need to support std::vector. The second is that as long as we're still using LvArray::Array<LvArray::Array>, we want those LvArray::Array<std::string> unit tests to pass and should work to maintain them.

Finally from an LvArray perspective if we want to only support trivial types and remove support for everything else that would be a major breaking change. It would certainly simplify the code a lot and make it easier to maintain but I'm also sure someone exists who would like to have a multi-dimensional array of non-trivial types.

[wrtobin]

Randy also pointed to streak (hardware-wise its essentially 1 node of lassen we're working using as a CI runner) which is having cuda memory errors with lvarray string arrays: https://github.com/GEOS-DEV/GEOS/actions/runs/7674651265/job/20919616872?pr=2873

Definitely agree with uniformity from a design perspective, though if we're encountering errors supporting a usage we don't need I'm fine losing uniformity to resolve the error.

The frontier issue seems to persist despite the experimental branch, so I think don't think we actually need to push for any changes to LvArray to drop support for non-trivial types (though I stand by that the string-typed lvarray unit tests are often what needs to be fixed when making changes and it could be a developer time savings to drop them -- though that would certainly be a breaking change).

[codecov[bot]]

Codecov Report

Attention: 95 lines in your changes are missing coverage. Please review.

Comparison is base (d306d3f) 51.44% compared to head (b1d3c21) 51.44%.

Files	Patch %	Lines
...coreComponents/dataRepository/BufferOps_inline.hpp	53.84%	12 Missing :warning:
...csSolvers/multiphysics/MultiphasePoromechanics.cpp	0.00%	7 Missing :warning:
...wavePropagation/AcousticElasticWaveEquationSEM.cpp	0.00%	7 Missing :warning:
...rmeability/TableRelativePermeabilityHysteresis.cpp	33.33%	4 Missing :warning:
...sSolvers/fluidFlow/CompositionalMultiphaseBase.cpp	0.00%	4 Missing :warning:
...sSolvers/multiphysics/SinglePhasePoromechanics.cpp	0.00%	4 Missing :warning:
src/coreComponents/dataRepository/BufferOps.hpp	50.00%	3 Missing :warning:
src/coreComponents/dataRepository/Group.cpp	70.00%	3 Missing :warning:
...fieldSpecification/EquilibriumInitialCondition.cpp	0.00%	3 Missing :warning:
src/coreComponents/mesh/CellElementRegion.hpp	0.00%	3 Missing :warning:
... and 26 more

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #2968   +/-   ##
========================================
  Coverage    51.44%   51.44%           
========================================
  Files          972      972           
  Lines        87065    87105   +40     
========================================
+ Hits         44788    44811   +23     
- Misses       42277    42294   +17     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[klevzoff]

The error can be seen here: https://github.com/GEOS-DEV/GEOS/actions/runs/7674651265/job/20919616872?pr=2873

I don't see any erros in that log that have anything to do with array<string>? There's a bunch of deallocation errors for array<double> though. They all begin after a segfault in testLifoStorage, so I would start with fixing that one first.

[paveltomin]

@rrsettgast abandon?

[rrsettgast]

@wrtobin What do you think? I still think maybe we should do this even though it wasn't the culprit I was looking for

[paveltomin]

@wrtobin What do you think? I still think maybe we should do this even though it wasn't the culprit I was looking for

@wrtobin @rrsettgast ping