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

Problem with FMI3 tests on Linux #63

Open
sagilar opened this issue Oct 2, 2024 · 4 comments
Open

Problem with FMI3 tests on Linux #63

sagilar opened this issue Oct 2, 2024 · 4 comments
Assignees
Labels
bug Something isn't working fmi3

Comments

@sagilar
Copy link

sagilar commented Oct 2, 2024

When executing cargo test on Linux in the branch dev/claudio, I get this error:

error[E0308]: mismatched types
   --> cli/tests/cli_tests_fmi3.rs:427:61
    |
427 | ...ce.get_interval_decimal(&[*vr], &mut interval, &mut qualifier).ok()....
    |       --------------------                        ^^^^^^^^^^^^^^ expected `&mut [u32]`, found `&mut [i32; 1]`
    |       |
    |       arguments to this method are incorrect
    |
    = note: expected mutable reference `&mut [u32]`
               found mutable reference `&mut [i32; 1]`

The original function looks this way:

fn get_interval_decimal(import: &Fmi3Import, cs_instance: &mut InstanceCS, vr: &u32) -> (f64, i32) {
    let mut interval: [f64; 1] = [0.0];
    let mut qualifier: [i32; 1] = [0];
    let error_msg = format!("get_interval_decimal failed for {}", vr);
    cs_instance.get_interval_decimal(&[*vr], &mut interval, &mut qualifier).ok().expect(&error_msg);

    (interval[0], qualifier[0])
}

The problem seems to be resolved by changing the data type of qualifier to u32 in line 425 and casting the return result to i32 in line 429 as follows:

fn get_interval_decimal(import: &Fmi3Import, cs_instance: &mut InstanceCS, vr: &u32) -> (f64, i32) {
    let mut interval: [f64; 1] = [0.0];
    let mut qualifier: [u32; 1] = [0]; // changed to u32 here
    let error_msg = format!("get_interval_decimal failed for {}", vr);
    cs_instance.get_interval_decimal(&[*vr], &mut interval, &mut qualifier).ok().expect(&error_msg);

    (interval[0], qualifier[0] as i32) // casting to i32 here
}

@clagms also got the same problem when running cargo test on Docker.

@sagilar sagilar added the bug Something isn't working label Oct 2, 2024
@sagilar sagilar added the fmi3 label Oct 2, 2024
@clagms
Copy link
Collaborator

clagms commented Oct 2, 2024

@YonVanom to reproduce the problem just run the dockerized build. I think it's caused by us being too strict on the types for the FMI functions... that's just a guess...

@YonVanom
Copy link
Collaborator

YonVanom commented Oct 2, 2024

This is a known issue related to the use of the rust-fmi library. It was previously encountered by Aleksander.
Here is some information from our discussion:

"As far as I understand, in C, the underlying datatype for an enum is typically an int, and should be an int32 on most systems. It’s only if there are too many enum values that the compiler can decide to make it an uint.

So as far as I understand, int32 should be correct."

"The rust-fmi library generates rust bindings from the standard fmi headers. It seems there is a difference when building for linux (in the docker) or for windows (local).

On windows I get this:

pub type fmi3IntervalQualifier = ::std::os::raw::c_int;

For linux (in the docker), I get this:

pub type fmi3IntervalQualifier = ::std::os::raw::c_uint;

The same is true for fmi3Status and fmi3DependencyKind, and also for the fmi2 bindings for fmi2Status, fmi2Type, and fmi2StatusKind."

"Seems to be a known “issue” when using bindgen. For OSX it will likely also result in c_uint:
rust-lang/rust-bindgen#1966
rust-lang/rust-bindgen#1907

Still not sure what to do with this.
We can use conditional compilation in cli_tests to handle this difference between platforms, e.g.:

#[cfg(target_os = "windows")]
pub type FmiIntervalQualifier = ::std::os::raw::c_int;

#[cfg(target_os = "linux")]
pub type FmiIntervalQualifier = ::std::os::raw::c_uint;

However, this raises the question if there are other issues that may arise. In UniFMU, the representation for these enums is explicitly made i32:

#[repr(i32)]
#[derive(Debug, PartialEq, Clone, Copy, IntoPrimitive, TryFromPrimitive)]
pub enum Fmi3IntervalQualifier {
    IntervalNotYetKnown = 0,
    IntervalUnchanged = 1,
    IntervalChanged = 2,
}

So, might this lead to other issues on platforms like linux/osx? I guess since these are predefined in the standard and there are only a handful of values, it shouldn’t?"

  • After this, @clagms suggested dropping rust-fmi:

"Should we drop this lib and just manually write the cbindings?

Seems we're encountering issues in second degree dependencies 😄.

This commit is where I deleted a bunch of code for loading and interacting with the FMU that doesn't use the rust-fmi package...: 2fdc44f

Do you think this is wise Yon?"

@clagms
Copy link
Collaborator

clagms commented Oct 2, 2024

Ah thank you! Then we've decided to drop rust_fmi, so we'll close this issue once we've migrated to fmpy @sagilar

@sagilar
Copy link
Author

sagilar commented Oct 2, 2024

Alright.
I will keep for now the changes which make the tests compile.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fmi3
Projects
None yet
Development

No branches or pull requests

3 participants