Add new FMU node-type

[riteshk001]

@al3xa23

New node type implementation based on Functional Mock-up Interface (FMI 3.0). Currently supports Co-Simulation. Functionality can be further extended to include Model Exchange simulation too.

[stv0g]

Nice work. Its great to see FMI support landing in VILLAS

[stv0g]

I think this not executing the FMU using a real-time scale?

Can you elaborate a bit how this node-type is indentend to be used?
E.g. does it expect periodic input data to proceed its internal time?
How does the internal time related to the real time?

Do we already have a PR for the node-type? I think this deserves some explanation.

[al3xa23]

Right now, it needs periodic input data. My plan would be to include real-time later

[pjungkamp]

I really like the general coding style, it's clean and very readable. Could you please fix the commit such that you have one commit that introduces the FMU node type. You have two unrelated changes changing the capitalization of writing_turn in opendss.{hpp,cpp} which should not be part of that commit. You can either move those changes into their commit or drop them.

[stv0g]

As per the new Contribution guidelines, could you please briefly state if LLMs have been used for this contribution?

[riteshk001]

The main logic was written by Jitpanu before. I modified some parts of the code myself and but I also used an LLM mainly to clean and format the code.

Edit: Sorry, I just saw the new Contribution guidelines regarding the use of LLMs,

• Cursor's LLM was used for formatting and cleaning the code in fmu.cpp.
• The same tool was also used to reformat the file to ensure variable naming adhered to the coding standards mentioned in the documentation.

[riteshk001]

Also, clang-format seems to format functions which have just a single line of body by rewriting it into a single line. I think that is why every time I ran clang-format, it would reformat the function body. I think that's why the code formatting looked different to what you expected.

For the integration test, I did not know where to place the fmu file in the repository, otherwise the intergration test itself is working. But once I add the fmu file, I can remove the "Test not ready" comment from the integration test.

[pjungkamp]

Just some more smaller nitpicks here.

[pjungkamp]

I'm happy with with the code. Run clang-format, squash it, and that should be fine.

integration test

On the topic of including an FMU in this repository for integration testing:

1. It must be redistributable under ASL2.
2. It must be linked against an old version glibc to ensure that it runs on all our supported systems. FMU's compiled on up-to-date distributions tend to have problems running with e.g. the glibc of older distributions.
3. I'd just put it next to the node-fmu.sh test script. Files with other file extensions than .sh won't be picked up by our integration test suite anyway. @stv0g Do you have a better place in mind?

I see an asine.fmu file in your integration test, where did that come from?

on the topic of binary blobs

We're linking our Nix builds against OpalRT's binary OpalOrchestra blobs, so I think that a binary FMU blob might be fine here, even if I don't like it. Binary blobs for testing are the way that the infamous xz backdoor was introduced into the otherwise open-source code.

I'd still like to see a way to build the FMU in a binary-reproducible fashion from source, but that tends to be rather hard. @stv0g We've recently discussed some better modelica/FMU tooling, do you have any good ideas here?

out of curiosity

I've taken a little more time to look into what the FMU actually does and have a few more questions. I don't want these to block the merge, you also don't have to change anything. Just some questions to satisfy my curiosity on some of your choices in the implementation.

1. fmu_write_first: Can you think of a reason to use fmu_write_first = false? This seems to me like it's using some default values specified inside the FMU description. There's also a default field on our signal lists that could be used to provide a default value, but it's not used much in VILLASnode.

2. fmi3_import_do_step: Your _read and _write implementations are currently running in lock-step, blocking on a condition-variable until the other function has finished its turn. The actual fmi3_import_do_step function which calls into the FMU to make progress is called as part of your _read implementation. Is there a reason for why you've opted to do this on _read rather than _write?

3. node/hook: It looks to me like the FMU is effectively a processing step. You translate our signals to the FMU inputs, do an FMU step, and read the output signals back out. This looks to me like this could potentially be implemented as a Hook::process function without any locking instead of a node-type with _read and _write running in lock-step. @stv0g What's your take here?

[riteshk001]

The asine.fmu file is a simple fmu model that I exported from a Simulink model of arcsine. I wanted to have a simple model to test the implementation and later include it in the integration test for the node. But there are also reference models that are given by Modelica (Reference FMUs) and licensed under BSD-2.

To answer your questions regarding the implementation:

1. The main reason I included the fmu_write_first was precisely to use the default values of the FMU model. The default values live inside the FMU model. Using the default field of the signal would potentially override the default values if the user supplied the wrong values, that's why I thought it was better to provide the option of directly using the default values provided by the FMU model.

2. I also wanted to follow the same style of implementation as done in node opendss. Since the idea was to pass input data to the FMU model whenever the node receives it, I felt it made sense to have it in the _read() call instead of the _write() call.

3. Is it possible for the Hook::process to have multiple signals coming in or going out? I may be wrong but it felt easier to have it implemented as a node because of the flexibility it offered when it comes to having multiple paths. I think having it as a node also makes it possible to expand on the functionality of FMU to have its own simulation time later?

[riteshk001]

I am a little confused with regards to variable naming conventions, the guidelines say that camel case is required for naming variables, functions and types. But across many cpp/hpp files, snake case is used instead. Which one should I stick to? :)

[pjungkamp]

I am a little confused with regards to variable naming conventions, the guidelines say that camel case is required for naming variables, functions and types. But across many cpp/hpp files, snake case is used instead. Which one should I stick to? :)

I know that pain. The old C code is pretty much entirely snake_case, so we've got a ton of free functions that are snake_case. We're also using encouraging use of the C++ standard library which also is entirely snake_case. Other libraries like fmt and spdlog also follow the style of the standard libaray, so we can't escape it. The C++ code introduced camelCase for member functions and virtual member functions of interfaces have permiated through the entire codebase.

I think that the current compromise is something like:

• local variables: snake_case
• non-static data members: snake_case
• free functions: snake_case
• member functions: mostly camelCase
• types: PascalCase

Don't use nested classes or type aliases unless it's really necessary.

Note that we can't enforce any of this statically. Take e.g. a std::back_inserter, if you want it to work with your type you need to have a snake_case push_back method. Or take the associated type aliases you should provide for custom iterator types like iterator_category.

And of course every dependency adds another naming convention into the mix...

[pjungkamp]

3. Is it possible for the Hook::process to have multiple signals coming in or going out? I may be wrong but it felt easier to have it implemented as a node because of the flexibility it offered when it comes to having multiple paths. I think having it as a node also makes it possible to expand on the functionality of FMU to have its own simulation time later?

The main limitations of a Hook compared to a loopback-style node like you have implemented here are:

• a hook can only process one sample at a time (no vectorization/batching)
• a hook can only output zero or one samples per input sample
• a hook can not produce an output without input

Using a node instead of a hook lifts these restrictions but forces the user to use 2 paths in VILLASnode, one for the input, one for the output side. Each path is a separate thread and thus adds extra synchronization overhead.

The current capabilities of the FMU node could well have been implemented as a Hook (as far as I can tell) but I don't know whether the expected advancements for e.g. model exchange and proper simulation time support would clash with the restrictions that a Hook imposes.

[pjungkamp]

The asine.fmu file is a simple fmu model that I exported from a Simulink model of arcsine. I wanted to have a simple model to test the implementation and later include it in the integration test for the node. But there are also reference models that are given by Modelica (Reference FMUs) and licensed under BSD-2.

Can you check the MathWorks/Simulink documentation about the licensing for FMUs exported from Simulink? I've never looked those that before.

A Modelica BSD-2 Clause FMU should be compatible with our Apache-2.0 license.

[stv0g]

I believe we should go with one of the Modelica Reference FMUs.

The Statespace one could be a good pick:

https://github.com/modelica/Reference-FMUs/tree/main/StateSpace

They are distributed as C Code which we could easily compile into an FMU without depending on external tools like Simulink or OpenModelica.

At the same time we have a good guarantee that they are Standard Compliant.

We could just built them from source or integrate them as a binary blob which we download. I would be open to both approaches.

But I agree with Philipp, that an integration test should be there.

[riteshk001]

I wanted to add the Statespace model too, but the outputs happen to be 3 element arrays which as of now are not supported by VILLAS if I am not wrong.
That's why I used the Dahlquist.fmu from the list of reference FMUs given by Modelica for the integration test, but it looks like the .fmu file itself is failing the license check by CI.

[pjungkamp]

Well, an FMU is just a ZIP file and thus can't be annotated using comments. If we assume a file named example.fmu you'd have to add an example.fmu.license file right next to the original one. This .license file can then include the SPDX-* annotations.

You can check the reuse.software website for the specification of how you should annotate uncommentable files.

[pjungkamp]

You're close! You'll have to run clang-format once more and fix your commit messages. They have a space between the type and the scope of the commit. Use type(scope): description instead of type (scope): description.

[pjungkamp]

You don't need C style comments here. Just put the SPDX headers there. Nothing else.

On a related note: Are you the author? I thought the FMU was a reference from Modelica sources.

[riteshk001]

Oh no, I did not make any modifications to the FMU, I was testing the license locally and forgot to remove it in the end. I have fixed it now. :)

[pjungkamp]

LGTM! @stv0g Can you take a short look?