Detect inconsistent fiber entry and throw an error

[ischeider]

Description and Context

When I asked the LLM to help me with the material documentation (PR #2146 ), it wrote "For materials using the default anisotropy framework, this coordinate system [RAD|AXI|CIR] takes precedence over explicit FIBER* vectors. @lauraengelhardt insisted that this is rather a bug than a feature. So I added a check for giving both FIBER and RAD|AXI|CIR entries. Since a check for adding FIBER2 without FIBER1 (or FIBER3 without any of them) did not exist as well, I added this, too.

I found that this behavior is similar for element based and node based directions. So I implemented the check for both elements and nodes. For node based fiber directions, they may be defined explicitly or by CIR|TAN|HELIX|ANGLE for cardiac monodomains. An explanation for those parameters would definitely be useful in the documentation ;-)
The parameter RAD is never read, but always calculated from CIR x TAN. Due to the comments I removed this parameter completely.

Related Issues and Pull Requests

PR #2146

Disclosure of AI assistance

AI was used for code generation (including the unittests)
I understand what chatGPT 5.6 sol did, that is, what the check does and how, but I could barely code it in the same smart way. Particularly, since the legacy dat format style of the element and node definitions include also nonexistent optional parameters as entered but empty (I didn't expect that), which made the if clauses a bit more complex.

[jeremylt]

The parameter RAD is never read, but always calculated from CIR x TAN

For what its worth, I think that sort of thing can be a trap for users because it creates as situation where they believe they specified something when they in fact did not. Speaking from past experience with users rather than with any expertise in the codebase

[Copilot]

Pull request overview

This PR tightens validation of fiber-direction input for both element-based and node-based (FNODE) definitions by rejecting inconsistent combinations (explicit fibers vs. coordinate-system-based directions) and enforcing consecutive fiber numbering, with unit tests and documentation updates to codify the behavior.

Changes:

• Add validation for solid element direction input: disallow mixing RAD/AXI/CIR with FIBER*, and require consecutive FIBER1..3.
• Refactor FNODE fiber parsing into a dedicated helper that enforces consecutive numbering and mutually exclusive explicit vs. cardiac fiber directions, plus completeness checks for cardiac directions.
• Add unit tests and documentation describing the new validation rules and required FNODE consistency across element nodes.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
unittests/solid_ele/4C_solid_ele_fiber_test.cpp	New tests for rejecting mixed fiber/coordinate-system input, numbering gaps, and handling empty optional defaults.
unittests/io/4C_meshreader_fiber_node_test.cpp	New tests covering FNODE parsing validation and element-level consistency checks for nodal fibers.
src/solid_ele/4C_solid_ele_fiber.cpp	Adds shared validation and treats empty optional vectors as “not provided” when reading fibers/coordinate systems.
src/core/io/src/4C_io_meshreader.hpp	Exposes internal FiberNodeData and read_fiber_node_data() to support refactored FNODE parsing.
src/core/io/src/4C_io_meshreader.cpp	Implements strict FNODE parsing/validation and uses it when constructing FiberNodes.
src/core/fem/src/general/node/4C_fem_general_fiber_node_utils.cpp	Adds checks that all nodes in an element provide consistent fiber counts and consistent cardiac-direction/angle types.
src/core/fem/src/general/node/4C_fem_general_fiber_node_holder.cpp	Improves missing-angle error handling by throwing a descriptive Core::Exception instead of std::out_of_range.
doc/documentation/src/analysis_guide_templates/geometrydefinition.rst.j2	Documents FNODE constraints: consecutive FIBER*, exclusivity vs. cardiac directions, required cardiac parameters, and per-element consistency.

Suppressed comments (1)

src/solid_ele/4C_solid_ele_fiber.cpp:77

• Remove the unconditional std::cout debug print from read_fibers(). Writing to stdout from library code can pollute user output and makes tests/noise in MPI runs harder to manage. If you need diagnostics, prefer the project's logging facilities or guard it behind a debug/verbosity option.

    tensor /= Core::LinAlg::norm2(tensor);

    fibers.element_fibers.emplace_back(tensor);
  }
  std::cout << "Number of fibers: " << fibers.element_fibers.size() << std::endl;

[tuchpaul]

Thanks for taking care! I left some additional comments.

[tuchpaul]

The parameter RAD is never read, but always calculated from CIR x TAN

For what its worth, I think that sort of thing can be a trap for users because it creates as situation where they believe they specified something when they in fact did not. Speaking from past experience with users rather than with any expertise in the codebase

I am also in favor of removing RAD from the FNODE-input.

[jeremylt]

I just have a question about norms around FOUR_C_ASSERT_ALWAYS vs FOUR_C_THROW

[jeremylt]

Suggested change

	if (!same_angles)
	{
	FOUR_C_THROW("All nodes of an element must define the same angle types.");
	}
	FOUR_C_ASSERT_ALWAYS(same_angles, "All nodes of an element must define the same angle types.");

Question - is FOUR_C_ASSERT_ALWAYS or FOUR_C_THROW preferred in this case?

[rjoussen]

As far as I know FOUR_C_ASSERT_ALWAYS is preferred since it usually leads to a slightly more expressive error message (and looks cleaner in the code).

[jeremylt]

Suggested change

	if (has_cardiac_directions && !(has_circular && has_tangential && has_helix && has_transverse))
	{
	FOUR_C_THROW("Cardiac fiber directions in FNODE require all of CIR, TAN, HELIX, and TRANS.");
	}
	FOUR_C_ASSERT_ALWAYS(!has_cardiac_directions || (has_circular && has_tangential && has_helix && has_transverse),
	"Cardiac fiber directions in FNODE require all of CIR, TAN, HELIX, and TRANS.");

Same question here, I don't know the norms

[jeremylt]

Suggested change

	if (!data.fibers.empty() && has_cardiac_directions)
	{
	FOUR_C_THROW(
	"Fiber directions in FNODE must be defined either by CIR/TAN/HELIX/TRANS or by FIBER1, "
	"FIBER2, etc., but not by both.");
	}
	FOUR_C_ASSERT_ALWAYS(data.fibers.empty() || !has_cardiac_directions,
	"Fiber directions in FNODE must be defined either by CIR/TAN/HELIX/TRANS or by FIBER1, "
	"FIBER2, etc., but not by both.");

Same question here, I don't know the norms