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

Add array size information to getters / setters #512

Open
t-sommer opened this Issue Dec 17, 2018 · 3 comments

Comments

Projects
None yet
5 participants
@t-sommer
Copy link
Collaborator

t-sommer commented Dec 17, 2018

Currently the getters and setters for variables only provide information about total size (nValues) of the memory passed to the function e.g.:

fmi3Status fmi3GetFloat64(fmi3Component c,
                          const fmi3ValueReference vr[], size_t nvr,
                          fmi3Float64 value[], size_t nValues);

However this makes it impossible to check the sizes of the individual arrays and provide meaningful error messages if they don't match. This is especially important if array sizes change during runtime.

My suggestion is to pass the sizes of the arrays as a parameter sizes[] (of length nvr):

fmi3Status fmi3GetFloat64(fmi3Component c,
                          const fmi3ValueReference vr[], size_t nvr,
                          fmi3Float64 value[], size_t sizes[]);

The total size of the memory can be calculated from the sizes[] by summing up it's values.

@t-sommer t-sommer added the needs poll label Dec 17, 2018

@t-sommer t-sommer added this to the FMI3.0 milestone Dec 17, 2018

@APillekeit

This comment has been minimized.

Copy link
Collaborator

APillekeit commented Jan 4, 2019

As far as we can remember this has already been discussed at an array working group meeting.

The nValues information is not really needed and could be deleted.

Based on our understanding the nValue information is a light weight information only intended for a basic check if the provided variable array is likely to be constructed correctly. Of course, this is not enough information to check the correctness of the internal array structure.

We would suggest keeping the light weight nValue information and avoid the overhead of providing complete structural information, since such thorough checks are not required in our view.

@masoud-najafi

This comment has been minimized.

Copy link

masoud-najafi commented Jan 7, 2019

As Andreas said, this has been already discussed. For any FMI API, the size of any vector should be given in the API arguments too.

@pmai pmai assigned pmai and unassigned pmai Jan 11, 2019

@chrbertsch

This comment has been minimized.

Copy link
Collaborator

chrbertsch commented Jan 11, 2019

Discussion on Meeting 1.11.2019
Karl: Was decided very early in the arrays group for FMI 2.1
Andreas J: the argument of Torsten was never discussed. If there is a mismatch, there is no way of providing diagnostic information. But goes contra the efficiency
Andreas P: overhead only for debugging (comment by Irina)
Andreas J: could be an argument with possible value NULL
Karl: Do not see the big overhead
Pierre: Do not know if people will use it the way it is intended. Do not know if people call for several variables simultaneously
Andreas J: If setting fails, one can use logging
Pierre: only significant benefit: multiple mismatches, that result in no error. Realisticly speaking this is only a constructed case. Most users will set arrays in separate calls (more efficient)
Andreas: is the combined call useless?
Pierre: Necessary for multiple scalars ....
Torsten S: also important for dynamic callers like Java (where function calls are expensive)
Andreas J: We need a node on this.
Pierre: I can do this

Christian: Regarding the ticket: if one needs debug information, one has to call separately, right?
Andreas: ...
Pierre: we do not have an error code when the sizes to not match
Andreas: In one case the FMU could give a good logging message, in the other case not
Pierre: in neither case we do require a specific error message in the case the sizes do not match
Andreas: The simulation will stop anyway; in one case one gets information what is not fitting.
Pierre: this would be an implementation fault of the either the importer or exporter; they could debug with different ways.
Pierre: we should have a specific error message to signal this to the importer
Torsten: wouldn't there be a lot of other places where this would be helpful?
Pierre: yes, is a separate issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment