Skip to content

Commit caece20

Browse files
authored
Merge pull request #85 from OpenSEMBA/copilot/fix-core-dump-issue
Prevent STEP import segfault and validate required input sections
2 parents 981b96a + 345ffc9 commit caece20

6 files changed

Lines changed: 104 additions & 11 deletions

File tree

docs/tulip_data_format.md

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
- [`shield`](#shield)
88
- [`dielectric`](#dielectric)
99
- [`open`](#open)
10-
- [`<model>`](#model)
10+
- [`<layers>`](#layers)
1111
- [.tulip.adapted.json file format](#tulipadaptedjson-file-format)
1212
- [Example](#example)
1313
- [`materials` array](#materials-array)
@@ -21,12 +21,14 @@ Tulip uses three types of file formats:
2121
- `CASE_NAME.tulip.out.json` which is the solver output containing the $L$ and $C$ PUL matrices for shielded domains and the multipolar expansion coefficients for an open domain.
2222

2323
# .tulip.input.json file format
24-
Tulip receives a JSON object as an input with the entries described below. Square brackets indicate that the entry is optional and a default value will be assumed, angle brackets indicate that the entry is mandatory.
24+
Tulip receives a JSON object as an input with the entries described below. Square brackets indicate that the entry is optional and a default value will be assumed, angle brackets indicate that the entry is mandatory.
2525

2626
Unless specified otherwise all units are assumed to be in SI-MKS.
2727

2828
Filename should be in the format `CASE_NAME.tulip.input.json`.
2929

30+
At minimum, the input JSON must include top-level `materials` and `layers` arrays.
31+
3032
## `[adapterOptions]`
3133
It can contain the following entries, as explained in [AdapterOption.h](../src/adapter/AdapterOptions.h) with their corresponding default values. An example is shown below.
3234
```json
@@ -49,7 +51,7 @@ Driver manages the solver`and generates outputs. Default options can be checked
4951
```
5052

5153
## `<materials>`
52-
These materials are associated with `model` `layers` to define regions with different material properties.
54+
These materials are associated with top-level `layers` to define regions with different material properties.
5355
They are defined by an array of JSON objects with:
5456
- `[name]` a string with a human readable name.
5557
- `<id>` an integer identifier with a unique number.
@@ -84,12 +86,11 @@ A dielectric is defined with a `[relativePermittivity]` which defaults to `1.0`.
8486
An `open` material serves to specify the computational boundary of the problem. It must intersect every other material layer. If no open boundary is specified for an open problem, one is computed automatically, together with _inner_ and _outer_ regions used to extract the unshielded multiwire coefficients.
8587

8688

87-
## `<model>`
88-
This object can contain the following entries:
89-
+ `<layers>` which is an array which associates the layers present in the `.step` file with the different `materials`. Each layer is specified by:
90-
- `<name>` which must match exactly the name of the corresponding layer within the `.step` file. It must be unique.
91-
- `<id>` which is an integer non-negative unique identifier which will be used to order the results for the calculated PUL matrices.
92-
- `<materialId>` which must match an `id` from a material in the list of `materials`
89+
## `<layers>`
90+
This top-level array associates the layers present in the `.step` file with the different `materials`. Each layer is specified by:
91+
- `<name>` which must match exactly the name of the corresponding layer within the `.step` file. It must be unique.
92+
- `<id>` which is an integer non-negative unique identifier which will be used to order the results for the calculated PUL matrices.
93+
- `<materialId>` which must match an `id` from a material in the list of `materials`
9394

9495

9596
# .tulip.adapted.json file format
@@ -177,4 +178,4 @@ This object can contain the following entries:
177178

178179
For unshielded-domains stores the parameters needed to reconstruct the field using a multipolar expansion.
179180

180-
It also stores `materialAssociation` information which serves to reconstruct the
181+
It also stores `materialAssociation` information which serves to reconstruct the

src/adapter/Adapter.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -566,6 +566,38 @@ void validateLayerMaterialIds(const nlohmann::json& inputJson)
566566
}
567567
}
568568

569+
void validateRequiredInputSections(const nlohmann::json& inputJson)
570+
{
571+
const bool hasMaterials =
572+
inputJson.contains("materials") && inputJson["materials"].is_array();
573+
const bool hasLayers = inputJson.contains("layers") && inputJson["layers"].is_array();
574+
575+
if (hasMaterials && hasLayers) {
576+
return;
577+
}
578+
579+
std::string missingSections;
580+
if (!hasMaterials) {
581+
missingSections += "'materials'";
582+
}
583+
if (!hasLayers) {
584+
if (!missingSections.empty()) {
585+
missingSections += " and ";
586+
}
587+
missingSections += "'layers'";
588+
}
589+
590+
std::string message =
591+
"Invalid input JSON: missing required top-level array section(s): " +
592+
missingSections + ".";
593+
if (inputJson.contains("model")) {
594+
message += " Found 'model'; expected top-level 'materials' and 'layers'.";
595+
} else {
596+
message += " Expected top-level 'materials' and 'layers'.";
597+
}
598+
throw std::runtime_error(message);
599+
}
600+
569601
std::vector<std::string> buildAcceptedStepNamesForLayer(
570602
const nlohmann::json& layer,
571603
const std::map<int, std::string>& materialTypeById)
@@ -799,6 +831,7 @@ void Adapter::initialize(const nlohmann::json& inputJson,
799831
caseName_ = caseName;
800832
inputDir_ = inputDir;
801833

834+
validateRequiredInputSections(inputJson);
802835
validateLayerMaterialIds(inputJson);
803836

804837
adapterOptions_ = parseAdapterOptions(inputJson, std::filesystem::path(inputDir_), caseName_);

src/adapter/ShapesClassification.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,7 @@ bool ShapesClassification::isOpenProblem() const
334334

335335
auto roots = nestedGraph.roots();
336336
if (open.size() == 1) return true;
337+
if (roots.empty()) return true;
337338
if (roots.size() > 1) return true;
338339
if (!roots.empty()) {
339340
const auto& root = roots[0];
@@ -415,7 +416,15 @@ EntityMap ShapesClassification::buildVacuumDomain() {
415416

416417
EntityMap ShapesClassification::buildClosedVacuumDomain() {
417418
const auto roots = nestedGraph.roots();
419+
if (roots.empty()) {
420+
throw std::runtime_error(
421+
"Unable to build closed vacuum domain: no root entity found.");
422+
}
418423
const auto& root = roots[0];
424+
if (!conductors.count(root)) {
425+
throw std::runtime_error(
426+
"Unable to build closed vacuum domain: root entity is not a conductor.");
427+
}
419428
EntityList dom = conductors.at(root);
420429
EntityList toRemove;
421430

test/adapter/AdapterTest.cpp

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,39 @@ TEST_F(AdapterTest, dielectric_unshielded_pair_fails_if_step_layer_is_not_presen
222222
std::runtime_error);
223223
}
224224

225+
TEST_F(AdapterTest, two_wires_open_fails_if_layers_section_is_missing)
226+
{
227+
const std::string caseName = "two_wires_open";
228+
nlohmann::json inputJson = readInputJsonFromCaseName(caseName);
229+
inputJson.erase("layers");
230+
inputJson["model"] = {{"materials", nlohmann::json::array()}};
231+
232+
try {
233+
Adapter adapter(inputJson, caseName, inputFolderFromCaseName(caseName));
234+
FAIL() << "Expected runtime_error";
235+
} catch (const std::runtime_error& err) {
236+
const std::string message = err.what();
237+
EXPECT_NE(message.find("'layers'"), std::string::npos);
238+
EXPECT_NE(message.find("top-level 'materials' and 'layers'"), std::string::npos);
239+
}
240+
}
241+
242+
TEST_F(AdapterTest, two_wires_open_fails_if_materials_section_is_missing)
243+
{
244+
const std::string caseName = "two_wires_open";
245+
nlohmann::json inputJson = readInputJsonFromCaseName(caseName);
246+
inputJson.erase("materials");
247+
248+
try {
249+
Adapter adapter(inputJson, caseName, inputFolderFromCaseName(caseName));
250+
FAIL() << "Expected runtime_error";
251+
} catch (const std::runtime_error& err) {
252+
const std::string message = err.what();
253+
EXPECT_NE(message.find("'materials'"), std::string::npos);
254+
EXPECT_NE(message.find("top-level 'materials' and 'layers'"), std::string::npos);
255+
}
256+
}
257+
225258
TEST_F(AdapterTest, dielectric_unshielded_pair)
226259
{
227260
const std::string caseName = "dielectric_unshielded_pair";
@@ -395,3 +428,20 @@ TEST_F(AdapterTest, overlapping_dielectrics_prioritize_higher_relative_permittiv
395428
EXPECT_NEAR(rightHighLeftMass, 4.0, 1e-9);
396429
EXPECT_NEAR(rightHighRightMass, 8.0, 1e-9);
397430
}
431+
432+
TEST_F(AdapterTest, shapes_classification_without_roots_is_treated_as_open_problem)
433+
{
434+
gmsh::clear();
435+
gmsh::model::add("no_roots_case");
436+
437+
const EntityList shapes = {};
438+
const nlohmann::json inputJson = {
439+
{"materials", nlohmann::json::array()},
440+
{"layers", nlohmann::json::array()}
441+
};
442+
443+
ShapesClassification classification(shapes, inputJson);
444+
445+
EXPECT_TRUE(classification.isOpenCase);
446+
EXPECT_TRUE(classification.isOpenProblem());
447+
}

testData/agrawal1981/agrawal1981.msh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29613,7 +29613,7 @@ $Nodes
2961329613
29597 -0.8995522719202876 1.763043536181375 0
2961429614
29598 -0.8877177445822432 1.769014368094762 0
2961529615
29599 -0.8854043705070319 1.781746044666594 0
29616-
29600 -0.8972652804878468 1.775827504646807 0
29616+
29600 -0.8972652804878468 1.775827504646806 0
2961729617
29601 -1.310615332880854 0.9476094369748136 0
2961829618
29602 -1.297944068191723 0.9458983500999457 0
2961929619
29603 -1.307559396848753 0.9695504897111458 0

0 commit comments

Comments
 (0)