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

Complex numbers (and other classes) do no show documentation string in Variables Browser #2455

Open
christiankral opened this issue Jan 30, 2018 · 4 comments
Labels
discussion Discussion issue that it not necessarily related to a concrete bug or feature

Comments

@christiankral
Copy link
Contributor

I recently noticed, that complex numbers lack a proper documentation when going through the variables browser. From my experience this applies both to OpenModelica and Dymola. The issue of not having documentation strings in the re-declaration of the real and imaginary part are resolved by #2443. However, the documentation string of the complex variable itself does not appear. As I understand from https://trac.openmodelica.org/OpenModelica/ticket/4721, this seems to be a flattening issue.

Consider the following example:

model Test "Test example"
  Modelica.SIunits.ComplexCurrent I = Complex(1,2) "Complex current";
end Test;

In OpenModelica the Variables Browser does neither show the documentation string of the complex variable nor of model Test:

complex_doc_string

I remember the discussions on the implementation of complex numbers. Even though I was always in favor of having a built in complex number type, one major argument against it always was, that the complex numbers are supposed to look like a built in type. Even the discussion in #1822 emphasizes this point of view. Yet documentation strings of complex numbers are not displayed..

On my opinion this issue needs discussion.

@christiankral christiankral added the discussion Discussion issue that it not necessarily related to a concrete bug or feature label Jan 30, 2018
@casella
Copy link
Contributor

casella commented Jan 31, 2018

As I understand it, this issue is a particular case of a more general problem, i.e., that the two mentioned tools only report the comments of flattened variables (I.re, I.im in this case), but not those of the enclosing class. This is particularly problematic in the case of Complex numbers, because the definition of a complex variable I contained in the comment is lost, but it could also lead to loss of information in other cases.

Consider the following example

model A
  Real x "Variable x";
  Real y "Variable y";
end A;

model B "My system"
  A a1 "Black model";
  A a2 "Red model"; 
end B;

When looking at the simulation results of model B, it would be nice to see something like

B        My System
|- a1     Black model
|  |- x    Variable x
|  |- y    Variable y
|
|- a2     Red model
   |- x    Variable x
   |- y    Variable y

otherwise the information about which is the Black model and which is the Red model, which is (legitimately) not conveyed by the instance names but just by their comments would be lost, hampering the interpretation of the simulation results. IMO, simulation results should also contain all the comments of enclosing classes for this purpose.

I understand this may be considered as a tool issue, but the issue raised by @christiankral is crucial and it would be nice to get some consensus (possibly some recommendations on the specification?) on the fact that tools are supposed to present this information to the end user.

@sjoelund
Copy link
Member

One limitation for this is the result-file format. We only store the comments for variables; not for classes (because they don't have any results available). This could of course be solved by either extending the format or using auxiliary files to give that information. But at least OpenModelica ignores the comments of classes since they are not used for anything. There is no attribute of im or re that depends on the comment or annotation of the class itself.

@christiankral
Copy link
Contributor Author

As the result-file format is simulation tool specific the feature of storing the class name is actually depending on the tool implementation. I totally agree with @casella that it makes sense to have the recommendation in the Modelica Language Specification on storing class annotations in the result file. Otherwise tool developers may not be aware that it makes sense to store and show this information in the result data for plotting and post processing.

Possibly @HansOlsson and others can also comment on this issue.

@HansOlsson
Copy link
Contributor

I don't see any major problem with supporting this, but the real problem is to support it with minimal impact - and that will require more thought.

I don't see a need for a specification change, as the specification doesn't say anything special about variables of simple types in this context (i.e. "Complex c" and "Real x" have the same type of documentation strings).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion issue that it not necessarily related to a concrete bug or feature
Projects
None yet
Development

No branches or pull requests

4 participants