Skip to content

Correct invalid names in journal files #2086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
c-p-schmidt wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Correct invalid names in journal files #2086

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d35f3b9
Add safety check to condition definition
c-p-schmidt Jun 16, 2026
c629aec
Adapt battery tutorial
c-p-schmidt Jun 12, 2026
3910505
Adapt fsi 2D tutorial
c-p-schmidt Jun 16, 2026
ce4676d
Adapt fsi 3D tutorial
c-p-schmidt Jun 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions src/core/fem/src/condition/4C_fem_condition_definition.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,12 @@

FOUR_C_NAMESPACE_OPEN

/* -----------------------------------------------------------------------------------------------*
| Class ConditionDefinition |
* -----------------------------------------------------------------------------------------------*/

/*----------------------------------------------------------------------*
*----------------------------------------------------------------------*/
Core::Conditions::ConditionDefinition::ConditionDefinition(std::string sectionname,
std::string conditionname, std::string description, Core::Conditions::ConditionType condtype,
bool buildgeometry, Core::Conditions::GeometryType gtype)
std::string conditionname, std::string description,
const Core::Conditions::ConditionType condtype, const bool buildgeometry,
const Core::Conditions::GeometryType gtype)
: sectionname_(std::move(sectionname)),
conditionname_(std::move(conditionname)),
description_(std::move(description)),
Expand Down Expand Up @@ -67,7 +66,7 @@ void Core::Conditions::ConditionDefinition::add_component(const Core::IO::InputS
/*----------------------------------------------------------------------*
*----------------------------------------------------------------------*/
void Core::Conditions::ConditionDefinition::read(
Core::IO::InputFile& input, std::vector<ConditionSpec>& condition_specs) const
const Core::IO::InputFile& input, std::vector<ConditionSpec>& condition_specs) const
{
Core::IO::InputParameterContainer container;
try
Expand All @@ -86,6 +85,13 @@ void Core::Conditions::ConditionDefinition::read(
{
auto parsed_condition_data = read_condition_data(condition_data);

if (parsed_condition_data.node_set_name.has_value())
{
FOUR_C_ASSERT_ALWAYS(parsed_condition_data.node_set_name.value().size() <= 32,
"NODE_SET_NAME '{}' exceeds the maximum length of 32 characters.",
parsed_condition_data.node_set_name.value());
}

Comment on lines +88 to +94

@rjoussen rjoussen Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this correctly, this now always throws an error if there is a condition definition in the input with a name longer than 32 characters. Isn't this overly restrictive? Gmsh and vtu meshes have no problem with reading meshes with long node set names. Maybe we shouldn't check that here, but in the Exodus reader itself, when a nodeset was read that has exactly 32 characters? Or, we only display a note about this problem when a nodeset has not been found by name, and it has more than 32 characters.

@c-p-schmidt c-p-schmidt Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also thought about that, but the problem is that I do not have the relevant information in the Exodus reader. One could only use the name length provided by the Exodus method as a proxy. If it has fewer than 32 characters, it was ok; if it has 32 characters, it might have been exactly the right length or too long. Which also felt like an unsatisfying solution to me.

However, you are right that this may be too strict for other mesh readers (I do not know). I just thought that 32 characters might be long enough anyway. But if you have doubts, we could only output a warning stating that for the Exodus reader, 32 characters is the maximum, and then the code would terminate later anyway, as the condition is not found in the mesh.

@rjoussen rjoussen Jun 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could only output a warning stating that for the Exodus reader, 32 characters is the maximum, and then the code would terminate later anyway, as the condition is not found in the mesh.

I would be in favor of that. This would only look a little strange if someone defines a name in exodus with exactly 32 characters and already knows about the limitation. In almost all scenarios, the user would see a warning about the length first and a crash immediately afterwards.

condition_specs.emplace_back(parsed_condition_data);
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/core/fem/src/condition/4C_fem_condition_definition.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,16 +116,16 @@ namespace Core::Conditions
\param condition_specs vector of the validated condition specifications in the input to be
filled.
*/
void read(Core::IO::InputFile& input, std::vector<ConditionSpec>& condition_specs) const;
void read(const Core::IO::InputFile& input, std::vector<ConditionSpec>& condition_specs) const;

/// name of my section in input file
std::string section_name() const { return sectionname_; }
[[nodiscard]] std::string section_name() const { return sectionname_; }

/// my condition name
std::string name() const { return conditionname_; }
[[nodiscard]] std::string name() const { return conditionname_; }

/// my GeometryType
Core::Conditions::GeometryType geometry_type() const { return gtype_; }
[[nodiscard]] Core::Conditions::GeometryType geometry_type() const { return gtype_; }

/// Get the InputSpec for this ConditionDefinition
[[nodiscard]] Core::IO::InputSpec spec() const;
Expand Down
Loading
Loading