[Core] Remove (or not) string-based geometry creation

[rubenzorrilla]

Description
Current ModelPart API allows for creating geometries with a string-based identifier. See below.

Kratos/kratos/includes/model_part.h

Lines 1481 to 1515 in e318076

	/**
	* @brief Inserts a geometry in the current model part.
	* @param rGeometryTypeName The type of the geometry to be added, must be registered.
	* @param rGeometryIdentifierName the identifier id of this geometry. Must be identical.
	* @param rGeometryNodeIds The node ids to create the geometry.
	*/
	GeometryType::Pointer CreateNewGeometry(
	const std::string& rGeometryTypeName,
	const std::string& rGeometryIdentifierName,
	const std::vector<IndexType>& rGeometryNodeIds
	);
	/**
	* @brief Inserts a geometry in the current model part.
	* @param rGeometryTypeName The type of the geometry to be added. Must be registered.
	* @param rGeometryIdentifierName the identifier id of this geometry. Must be identical.
	* @param pGeometryNodes The nodes array to create the geometry.
	*/
	GeometryType::Pointer CreateNewGeometry(
	const std::string& rGeometryTypeName,
	const std::string& rGeometryIdentifierName,
	GeometryType::PointsArrayType pGeometryNodes
	);
	/**
	* @brief Inserts a geometry in the current model part.
	* @param rGeometryTypeName The type of the geometry to be added. Must be registered.
	* @param rGeometryIdentifierName the identifier id of this geometry. Must be identical.
	* @param pGeometry The pointer to an existing geometry.
	*/
	GeometryType::Pointer CreateNewGeometry(
	const std::string& rGeometryTypeName,
	const std::string& rGeometryIdentifierName,
	GeometryType::Pointer pGeometry
	);

Such string is then converted to a hash identifier while constructing the Geometry. As far as I see, there are no current usages of this. On top of enlarging the already exorbitant ModelPart API, in my opinion this adds an extra complexity with no clear value. Taking into account that we're working on and promoting the geometry-based I/O, I'm opening this issue to discuss if we should whether keep or remove this feature.

In case it is required, this was added in #6335.

[loumalouomega]

I mentioned this in the original PR and there was a reason @tteschemacher

[tteschemacher]

Hey.

I think it is actually a quite good feature and we also spent some efforts to make it very efficient.

The reason behind this is that when coming from cad or generally non fem environments there may be no clear identification system. However often you have something like surface 1, 2, 3, ... Curve 1, 2, ... Same actually works with guids. These are appearing often.

As I recall CAE beta had that issue.with text names.

I'm not insisting on keeping it though.

[loumalouomega]

If at the end we are using an Id (integer), we can move the feature as an utility, so advanced developers can use it if interested.

[rubenzorrilla]

Hey.

I think it is actually a quite good feature and we also spent some efforts to make it very efficient.

The reason behind this is that when coming from cad or generally non fem environments there may be no clear identification system. However often you have something like surface 1, 2, 3, ... Curve 1, 2, ... Same actually works with guids. These are appearing often.

As I recall CAE beta had that issue.with text names.

I'm not insisting on keeping it though.

This makes very much sense. However, I wonder if such capability must be in the I/O rather than in the ModelPart and Geometry classes. In other words, I think that Kratos should always work with an integer-based Id but be able to parse (and convert) string-based ones. Nevertheless, at this point I think it would be easier to keep current implementation.

[loumalouomega]

Hey.
I think it is actually a quite good feature and we also spent some efforts to make it very efficient.
The reason behind this is that when coming from cad or generally non fem environments there may be no clear identification system. However often you have something like surface 1, 2, 3, ... Curve 1, 2, ... Same actually works with guids. These are appearing often.
As I recall CAE beta had that issue.with text names.
I'm not insisting on keeping it though.

This makes very much sense. However, I wonder if such capability must be in the I/O rather than in the ModelPart and Geometry classes. In other words, I think that Kratos should always work with an integer-based Id but be able to parse (and convert) string-based ones. Nevertheless, at this point I think it would be easier to keep current implementation.

Can we close this then?

[rubenzorrilla]

Hey.
I think it is actually a quite good feature and we also spent some efforts to make it very efficient.
The reason behind this is that when coming from cad or generally non fem environments there may be no clear identification system. However often you have something like surface 1, 2, 3, ... Curve 1, 2, ... Same actually works with guids. These are appearing often.
As I recall CAE beta had that issue.with text names.
I'm not insisting on keeping it though.

This makes very much sense. However, I wonder if such capability must be in the I/O rather than in the ModelPart and Geometry classes. In other words, I think that Kratos should always work with an integer-based Id but be able to parse (and convert) string-based ones. Nevertheless, at this point I think it would be easier to keep current implementation.

Can we close this then?

I think we can but let me leave it open for a while to communicate it to the rest of the @KratosMultiphysics/technical-committee .

[pooyan-dadvand]

@KratosMultiphysics/technical-committee agrees with the use case of @tteschemacher and will keep it in modelpart as it is. This also gives the possibility to eventually add a reverse dictionary if is needed.