ModelicaStandardLibrary icon indicating copy to clipboard operation
ModelicaStandardLibrary copied to clipboard

remove unused code (#3622)

Open adrpo opened this issue 5 years ago • 9 comments

There are some components defined that are not used in the blocks or functions. This PR removes them.

adrpo avatar Aug 29 '20 23:08 adrpo

Removing protected parameters (even if unused) seems problematic with current versioning rules.

HansOlsson avatar Aug 30 '20 09:08 HansOlsson

Removing protected parameters (even if unused) seems problematic with current versioning rules.

Indeed. It is not covered by conversion rules. For that reason, we mentioned every removed component as backward-compatibility breaking change in the release notes and require a new major version.

beutlich avatar Aug 30 '20 17:08 beutlich

So we could remove the protected parameters from functions, as these quantities cannot be accessed from outside and keep them in models and blocks. Additionally, all commented code fragments shall be removed.

christiankral avatar Aug 31 '20 07:08 christiankral

See https://github.com/adrpo/Modelica/pull/1

christiankral avatar Aug 31 '20 08:08 christiankral

@AHaumer @adrpo Would you agree to better document the code lines? I understand that @ahaumer wants to keep the commented code as it represents an alternative (inverse) representation of the used equations.

Example: Change

  Real RotationMatrix[2, 2]={{+cos(-angle),-sin(-angle)},{+sin(-angle),+
      cos(-angle)}};
  //Real InverseRotator[2,2] = {{+cos(+angle),-sin(+angle)},{+sin(+angle),+cos(+angle)}};
  ...
  //u = InverseRotator*y;

to

  Real RotationMatrix[2, 2]={{+cos(-angle),-sin(-angle)},{+sin(-angle),+
      cos(-angle)}};
  // Alternative equivalent implementation:
  // Real InverseRotator[2,2] = {{+cos(+angle),-sin(+angle)},{+sin(+angle),+cos(+angle)}};
  ...
  // Alternative equivalent implementation:
  // u = InverseRotator*y;

christiankral avatar May 16 '21 21:05 christiankral

Is OK from my side, I thought it was some left over code, but if this is documentation, of course we should keep it. @AHaumer just let us know how you want it.

adrpo avatar May 17 '21 07:05 adrpo

I'd prefer to keep the comments as internal documentation - thanks.

AHaumer avatar May 24 '21 09:05 AHaumer

I'd prefer to keep the comments as internal documentation - thanks.

@AHaumer OK, but shall we add

// Alternative equivalent implementation:

as proposed above to make the documentation character more clear to the users?

christiankral avatar Jun 26 '21 15:06 christiankral

@AHaumer OK, but shall we add

// Alternative equivalent implementation:

as proposed above to make the documentation character more clear to the users?

Fine with me.

AHaumer avatar Dec 13 '21 15:12 AHaumer

@AHaumer @christiankral, we have made the Alternative equivalent implementation comment in relevant places. Could you please review?

arunkumar-narasimhan avatar Jan 18 '24 06:01 arunkumar-narasimhan