Skip to content
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.

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

Correct error check on causality="independent" and start attribute(s) #129

Merged
merged 4 commits into from
Jul 17, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
3 changes: 3 additions & 0 deletions Test/FMI2/parser_test_xmls/variable_test/modelDescription.xml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@
<ScalarVariable name="derivative" causality="output" variability="continuous" valueReference="10" initial="approx" description="myDescription">
<Real derivative="1" start="1.0"/>
</ScalarVariable>
<ScalarVariable causality="independent" description="time" name="time" valueReference="100" variability="continuous">
<Real/>
</ScalarVariable>
</ModelVariables>

<ModelStructure>
Expand Down
35 changes: 35 additions & 0 deletions Test/FMI3/fmi3_variability_causality_initial_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -418,3 +418,38 @@ TEST_CASE("Test missing start values") {
REQUIRE(fmi3_testutil_get_num_problems(tfmu) == 6);
fmi3_testutil_import_free(tfmu);
}

TEST_CASE("Test independent variable with start value") {
const char* xmldir = FMI3_TEST_XML_DIR "/variability_causality_initial/invalid/independent_with_start";
fmi3_testutil_import_t* tfmu = fmi3_testutil_parse_xml_with_log(xmldir);
REQUIRE(tfmu != nullptr);
fmi3_import_t* fmu = tfmu->fmu;
REQUIRE(fmu != nullptr);

REQUIRE(fmi3_testutil_log_contains(tfmu, "Variable 'f64_indep': The independent variable is not allowed to have a start attribute."));

REQUIRE(fmi3_testutil_get_num_problems(tfmu) == 1);
fmi3_testutil_import_free(tfmu);
}

// TODO: See comments in fmi3_log_warnings_start_exists function
/*
TEST_CASE("Test initial == 'calculated' with start value") {
const char* xmldir = FMI3_TEST_XML_DIR "/variability_causality_initial/invalid/initial_calculated_with_start";
fmi3_testutil_import_t* tfmu = fmi3_testutil_parse_xml_with_log(xmldir);
REQUIRE(tfmu != nullptr);
fmi3_import_t* fmu = tfmu->fmu;
REQUIRE(fmu != nullptr);

REQUIRE(fmi3_testutil_log_contains(tfmu, "Variable 'f64': Variables with initial == \"calculated\" are not allowed to have a start attribute."));
REQUIRE(fmi3_testutil_log_contains(tfmu, "Variable 'i64': Variables with initial == \"calculated\" are not allowed to have a start attribute."));
REQUIRE(fmi3_testutil_log_contains(tfmu, "Variable 'bool': Variables with initial == \"calculated\" are not allowed to have a start attribute."));
REQUIRE(fmi3_testutil_log_contains(tfmu, "Variable 'binary': Variables with initial == \"calculated\" are not allowed to have a start attribute."));
REQUIRE(fmi3_testutil_log_contains(tfmu, "Variable 'binary_arr': Variables with initial == \"calculated\" are not allowed to have a start attribute."));
REQUIRE(fmi3_testutil_log_contains(tfmu, "Variable 'string': Variables with initial == \"calculated\" are not allowed to have a start attribute."));
REQUIRE(fmi3_testutil_log_contains(tfmu, "Variable 'enumDefault': Variables with initial == \"calculated\" are not allowed to have a start attribute."));

REQUIRE(fmi3_testutil_get_num_problems(tfmu) == 7);
fmi3_testutil_import_free(tfmu);
}
*/
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8"?>
<fmiModelDescription fmiVersion="3.0" modelName="" instantiationToken="">
<CoSimulation modelIdentifier="id" />

<ModelVariables>
<!-- "If initial = calculated or causality = independent, it is not allowed to provide a start attribute." -->
<Float64 name="f64_indep" valueReference="1" causality="independent" start="0"/>
</ModelVariables>

<ModelStructure/>
</fmiModelDescription>
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?xml version="1.0" encoding="UTF-8"?>
<fmiModelDescription fmiVersion="3.0" modelName="" instantiationToken="">
<CoSimulation modelIdentifier="id" />

<TypeDefinitions>
<EnumerationType name="MyEnum" quantity="TypeQuantity">
<Item name="item1" value="1"/>
</EnumerationType>
</TypeDefinitions>

<ModelVariables>
<!-- "If initial = calculated or causality = independent, it is not allowed to provide a start attribute." -->
<Float64 name="f64" valueReference="1" initial="calculated" start="0"/>
<Int64 name="i64" valueReference="2" initial="calculated" start="0"/>
<Boolean name="bool" valueReference="3" initial="calculated" start="true"/>
<Binary name="binary" valueReference="4" initial="calculated">
<Start value="0011BBff029eE4Cd"/>
</Binary>
<Binary name="binary_arr" valueReference="5" initial="calculated">
<Dimension start="1"/>
<Start value="0011BBff029eE4Cd"/>
</Binary>
<String name="string" valueReference="6" initial="calculated">
<Start value="abc"/>
</String>
<Enumeration name="enumDefault" valueReference="7" declaredType="MyEnum" initial="calculated" start="1"/>
</ModelVariables>

<ModelStructure/>
</fmiModelDescription>
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@
<Float64 name="f64_input" valueReference="4" start="0" causality="input"/>
<Float64 name="f64_output" valueReference="5" start="0" causality="output"/>
<Float64 name="f64_local" valueReference="6" start="0" causality="local"/>
<Float64 name="f64_independent" valueReference="7" start="0" causality="independent"/>
<Float64 name="f64_independent" valueReference="7" causality="independent"/>

<Int64 name="i64_structural_parameter" valueReference="11" start="0" causality="structuralParameter"/>
<Int64 name="i64_parameter" valueReference="12" start="0" causality="parameter"/>
<Int64 name="i64_calculated_parameter" valueReference="13" start="0" causality="calculatedParameter"/>
<Int64 name="i64_input" valueReference="14" start="0" causality="input"/>
<Int64 name="i64_output" valueReference="15" start="0" causality="output"/>
<Int64 name="i64_local" valueReference="16" start="0" causality="local"/>
<Int64 name="i64_independent" valueReference="17" start="0" causality="independent"/>
<Int64 name="i64_independent" valueReference="17" causality="independent"/>
</ModelVariables>

<ModelStructure/>
Expand Down
4 changes: 2 additions & 2 deletions src/XML/src/FMI2/fmi2_xml_variable.c
Original file line number Diff line number Diff line change
Expand Up @@ -479,8 +479,8 @@ int fmi2_xml_handle_ScalarVariable(fmi2_xml_parser_context_t *context, const cha

int fmi2_xml_get_has_start(fmi2_xml_parser_context_t *context, fmi2_xml_variable_t* variable) {
int hasStart = fmi2_xml_is_attr_defined(context, fmi_attr_id_start);
if(!hasStart) {
if (variable->initial != (char)fmi2_initial_enu_calculated) {
if (!hasStart) {
PeterMeisrimelModelon marked this conversation as resolved.
Show resolved Hide resolved
if ((variable->initial != (char)fmi2_initial_enu_calculated) && (variable->causality != (char)fmi2_causality_enu_independent)) {
fmi2_xml_parse_error(context,
"Start attribute is required for this causality, variability and initial combination");
hasStart = 1;
Expand Down
47 changes: 36 additions & 11 deletions src/XML/src/FMI3/fmi3_xml_variable.c
Original file line number Diff line number Diff line change
Expand Up @@ -1559,7 +1559,7 @@ int fmi3_xml_handle_Variable(fmi3_xml_parser_context_t* context, const char* dat
return 0;
}

static void fmi3_log_error_if_start_required(fmi3_xml_parser_context_t* context, fmi3_xml_variable_t* variable) {
static void fmi3_log_warning_if_start_required(fmi3_xml_parser_context_t* context, fmi3_xml_variable_t* variable) {
// causality checks
if (variable->causality == fmi3_causality_enu_input) {
fmi3_xml_parse_warning(context, "Variable '%s': start value required for input variables",
Expand All @@ -1584,6 +1584,24 @@ static void fmi3_log_error_if_start_required(fmi3_xml_parser_context_t* context,
}
}

static void fmi3_log_warnings_start_exists(fmi3_xml_parser_context_t* context, fmi3_xml_variable_t* variable) {
/* If initial = calculated or causality = independent, it is not allowed to provide a start attribute. */
/* The variable is calculated by the FMU from other variables during initialization. For calculated variables a start attribute must not be provided. */
// TODO: This one is quite a bit more delicate to properly implement, since the default inferred initial may cause issues here.
// E.g., Float with causality = output + start values
// variability = continuous is inferred, default initial for coninuous outputs is 'calculated', would need to choose non-default, but valid initial
/*
if (variable->initial == fmi3_initial_enu_calculated) {
fmi3_xml_parse_warning(context, "Variable '%s': Variables with initial == \"calculated\" are not allowed to have a start attribute.",
fmi3_xml_get_variable_name(variable));
}
*/
if (variable->causality == fmi3_causality_enu_independent) {
fmi3_xml_parse_warning(context, "Variable '%s': The independent variable is not allowed to have a start attribute.",
fmi3_xml_get_variable_name(variable));
}
}

int fmi3_xml_handle_Dimension(fmi3_xml_parser_context_t* context, const char* data) {
if (!data) { /* start of tag */
fmi3_xml_variable_t* currentVar = jm_vector_get_last(jm_voidp)(&context->modelDescription->variablesOrigOrder);
Expand Down Expand Up @@ -1730,7 +1748,7 @@ int fmi3_xml_handle_FloatXX(fmi3_xml_parser_context_t* context, const char* data
// TODO: Can one get these from some sort of defaults instead?
start->start.array64s = NULL;
start->start.array32s = NULL;
fmi3_log_error_if_start_required(context, variable);
fmi3_log_warning_if_start_required(context, variable);
}
} else { /* is scalar */
// TODO:
Expand All @@ -1746,8 +1764,9 @@ int fmi3_xml_handle_FloatXX(fmi3_xml_parser_context_t* context, const char* data
}
}
variable->type = &start->super;
fmi3_log_warnings_start_exists(context, variable);
} else {
fmi3_log_error_if_start_required(context, variable);
fmi3_log_warning_if_start_required(context, variable);
}

/* TODO: verify that dimension size == number of array start attribute values */
Expand Down Expand Up @@ -1817,7 +1836,7 @@ int fmi3_xml_handle_IntXX(fmi3_xml_parser_context_t* context, const char* data,
start->start.array32u = NULL;
start->start.array16u = NULL;
start->start.array8u = NULL;
fmi3_log_error_if_start_required(context, variable);
fmi3_log_warning_if_start_required(context, variable);
}
} else { /* is scalar */
/* restore the attribute buffer before it's used in set_attr_int */
Expand All @@ -1834,8 +1853,9 @@ int fmi3_xml_handle_IntXX(fmi3_xml_parser_context_t* context, const char* data,
}
}
variable->type = &start->super;
fmi3_log_warnings_start_exists(context, variable);
} else {
fmi3_log_error_if_start_required(context, variable);
fmi3_log_warning_if_start_required(context, variable);
}
}
return 0;
Expand Down Expand Up @@ -1905,7 +1925,7 @@ int fmi3_xml_handle_Boolean(fmi3_xml_parser_context_t *context, const char* data
start->start.array32u = NULL;
start->start.array16u = NULL;
start->start.array8u = NULL;
fmi3_log_error_if_start_required(context, variable);
fmi3_log_warning_if_start_required(context, variable);
}
} else {
/* restore the attribute buffer before it's used in set_attr_boolean */
Expand All @@ -1917,8 +1937,9 @@ int fmi3_xml_handle_Boolean(fmi3_xml_parser_context_t *context, const char* data
}
}
variable->type = &start->super;
fmi3_log_warnings_start_exists(context, variable);
} else {
fmi3_log_error_if_start_required(context, variable);
fmi3_log_warning_if_start_required(context, variable);
}
return 0;
}
Expand Down Expand Up @@ -1992,10 +2013,11 @@ int fmi3_xml_handle_Binary(fmi3_xml_parser_context_t* context, const char* data)
}

variable->type = &startObj->super;
fmi3_log_warnings_start_exists(context, variable);
// Resize the vector to 0 since we are now done with the current variable.
jm_vector_resize(jm_voidp)(&context->currentStartVariableValues, 0);
} else {
fmi3_log_error_if_start_required(context, variable);
fmi3_log_warning_if_start_required(context, variable);
}
}
return 0;
Expand Down Expand Up @@ -2079,8 +2101,9 @@ int fmi3_xml_handle_String(fmi3_xml_parser_context_t *context, const char* data)
if (!fmi3_xml_variable_is_array(variable) && (nStart != 1)) {
fmi3_xml_parse_warning(context, "Variable %s: Found %" PRIu64 " Start elements for non-array variable", variable->name, nStart);
}
fmi3_log_warnings_start_exists(context, variable); // only captures binary arrays
} else {
fmi3_log_error_if_start_required(context, variable);
fmi3_log_warning_if_start_required(context, variable);
}
}
return 0;
Expand Down Expand Up @@ -2165,6 +2188,7 @@ int fmi3_xml_handle_BinaryVariableStart(fmi3_xml_parser_context_t* context, cons
return -1;
}
variable->type = &startObj->super;
fmi3_log_warnings_start_exists(context, variable); // binary non-array
}
}
return 0;
Expand Down Expand Up @@ -2270,7 +2294,7 @@ int fmi3_xml_handle_Enumeration(fmi3_xml_parser_context_t *context, const char*
start->start.array32u = NULL;
start->start.array16u = NULL;
start->start.array8u = NULL;
fmi3_log_error_if_start_required(context, variable);
fmi3_log_warning_if_start_required(context, variable);
}
} else {
/* restore the attribute buffer before it's used in set_attr_int */
Expand All @@ -2282,8 +2306,9 @@ int fmi3_xml_handle_Enumeration(fmi3_xml_parser_context_t *context, const char*
}
}
variable->type = &start->super;
fmi3_log_warnings_start_exists(context, variable);
} else {
fmi3_log_error_if_start_required(context, variable);
fmi3_log_warning_if_start_required(context, variable);
}
}
return 0;
Expand Down