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

Clarify variability of functions. #2924

Merged
merged 20 commits into from Nov 10, 2021

Conversation

HansOlsson
Copy link
Collaborator

Closes #2538

@henrikt-ma henrikt-ma self-requested a review May 2, 2021 21:11
Copy link
Collaborator

@henrikt-ma henrikt-ma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to pursue this view, I think the variability rules must be updated accordingly.

I also think that at least the function-compatibility definition must be refined to consider variability of any binding assignments.

I'm also wondering if one has to do it this way, considering how non-transparent it makes the variability of a function call. It would be good if a complicated design was backed up by real-world examples in need of it.

@qlambert-pro
Copy link
Collaborator

qlambert-pro commented May 3, 2021

I also don't think it fixes:

model TestModel
  function f
    input Boolean c;
    input Real i = if c then g(c) else 1;
    output Real o = i;
  end f;

  function g
    input Boolean c;
    input Real i = if c then f(c) else 1;
    output Real o = i;
  end g;

  constant Real c = g(false);
end TestModel;

unless you expect evaluation to happen before you try to compute the variability of an expression?

@HansOlsson
Copy link
Collaborator Author

I also don't think it fixes:

model TestModel
  function f
    input Boolean c;
    input Real i = if c then g(c) else 1;
    output Real o = i;
  end f;

  function g
    input Boolean c;
    input Real i = if c then f(c) else 1;
    output Real o = i;
  end g;

  constant Real c = g(false);
end TestModel;

unless you expect evaluation to happen before you try to compute the variability of an expression?

Exactly, it's not legal, similarly as the following isn't:

model TestModel
  function f
    input Boolean c;
    input Real i = g(c);
    output Real o = i;
  end f;

  function g
    input Boolean c;
    input Real i = f(c);
    output Real o = i;
  end g;

  constant Real c = g(false);
end TestModel;

I we want to pursue this view, I think the variability rules must be updated accordingly.

That is a very esoteric case, when you have a constant/parameter bound to a replaceable function - and then redeclare that function to a function with additional defaults of higher variability.

@qlambert-pro
Copy link
Collaborator

qlambert-pro commented May 3, 2021

Exactly, it's not legal,

The problem is not the legality, it is the computability. As in, you can't compute the variability of f(c) and g(c) because they are mutually dependent. So you can't even reject it because you algorithm does not finish.

@HansOlsson
Copy link
Collaborator Author

HansOlsson commented May 3, 2021

Exactly, it's not legal,

The problem is not the legality, it is the computability. As in, you can't compute the variability of f(c) and g(c) because they are mutually dependent. So you can't even reject it because you algorithm does not finish.

But then we can just look at:

model TestModel
  function g
    input Boolean c;
    input Real i = g(c);
    output Real o = i;
  end g;

  constant Real c = g(false);
end TestModel;

and conclude that the underlying issue is that the function-defaults use a recursive call; and add a restriction saying that the function defaults may not recursively call the function.

@qlambert-pro
Copy link
Collaborator

qlambert-pro commented May 3, 2021

That would work although I think having to do a prior analysis of the existing recursive dependency between functions and how these depend on the default-binding of a given argument to be able to finally check something as straight forward as the variability of an expression is disappointing.

How would the following case be handled:

model M
  function foo
    output Real y;
  protected
    Real z;
  algorithm
    y:=z;
  end foo;
  Real x=time;
  function f1=foo(z=x);
  constant Real z=f1(); //Illegal because x is not constant.
end M;

@HansOlsson
Copy link
Collaborator Author

That would work although I think having to do a prior analysis of the existing recursive dependency between functions and how these depend on the default-binding of a given argument to be able to finally check something as straight forward as the variability of an expression is disappointing.

That seems normal as analysis of a function depend on the function being correct in some way.

How would the following case be handled:

model M
  function foo
    output Real y;
  protected
    Real z;
  algorithm
    y:=z;
  end foo;
  Real x=time;
  function f1=foo(z=x);
  constant Real z=f1(); //Illegal because x is not constant.
end M;

I hope you mean another input and not a protected variable, since that cannot be modified.
The call of f1 will be continuous-time due to the default argument, similarly as f1(z=x); has continuous-time variability.

@HansOlsson
Copy link
Collaborator Author

I hope you mean another input and not a protected variable, since that cannot be modified.
The call of f1 will be continuous-time due to the default argument, similarly as f1(z=x); has continuous-time variability.

If we really care about whether a replaceable function will be constant with arguments we could introduce some variant of "replaceable constant function", inspired by "constexpr" functions in C++ (but with different meaning); meaning that it is constant if the inputs are constant.

@qlambert-pro
Copy link
Collaborator

I hope you mean another input and not a protected variable, since that cannot be modified.

I forgot about that.
Is the following case covered then:

model M
  model N
    replaceable function foo
      output Real y;
    protected
      Real z;
    algorithm
      y := z;
    end foo;
  end N;

  extends N;

  redeclare function extends foo(z = x)
  end foo;

  Real x = time;
  constant Real z = foo();
end M;

@qlambert-pro
Copy link
Collaborator

qlambert-pro commented May 3, 2021

If we really care about whether a replaceable function will be constant with arguments we could introduce some variant of "replaceable constant function", inspired by "constexpr" functions in C++ (but with different meaning); meaning that it is constant if the inputs are constant.

The solution we talked about at the time was to have a variability prefix for function definitions that behaved in that way.
Meaning that the variability of the default-binding could be checked against the variability of the function, then the specified inputs and the function variability would be enough to know the variability of a call.

@HansOlsson
Copy link
Collaborator Author

If we really care about whether a replaceable function will be constant with arguments we could introduce some variant of "replaceable constant function", inspired by "constexpr" functions in C++ (but with different meaning); meaning that it is constant if the inputs are constant.

The solution we talked about at the time was to have a variability prefix for function definitions that behaved in that way.
Meaning that the variability of the default-binding could be checked against the variability of the function, then the specified inputs and the function variability would be enough to know the variability of a call.

It's certainly possible, but it will:

  • require a change of the grammar.
  • require an update of the checking rules
  • require an update of libraries such as MSL since Modelica.Fluid.Machines.BaseClasses.PartialPump uses flowCharacteristic to compute a final parameter. There might be more such cases, and it might be that we could change it to be a final non-parameter. Note that flowCharacteristic is redeclared with modifiers in this way (but they are parameters so it checks out).

Additionally the idea of "constant function" and "parameter function" will break down for "evaluable parameter function".

I see the following variants:

  • Accept this proposal as it was.
  • Add that variability of calls of redeclared functions isn't covered by sub-type checking.
  • Accept this proposal as is, and make the more complex changes in a separate PR.
  • Add the more complex changes in this PR.

I added text for the second variant to make it more concrete. We can always revert that.

Copy link
Collaborator

@henrikt-ma henrikt-ma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking review as request changes, but what I really mean by this is that I think we need more active participation from all tool vendors on this matter.

chapters/interface.tex Outdated Show resolved Hide resolved
chapters/functions.tex Outdated Show resolved Hide resolved
chapters/functions.tex Outdated Show resolved Hide resolved
chapters/functions.tex Outdated Show resolved Hide resolved
chapters/functions.tex Outdated Show resolved Hide resolved
chapters/functions.tex Outdated Show resolved Hide resolved
chapters/interface.tex Outdated Show resolved Hide resolved
chapters/interface.tex Outdated Show resolved Hide resolved
chapters/interface.tex Outdated Show resolved Hide resolved
Copy link
Member

@eshmoylova eshmoylova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With suggested changes it looks good to me.

chapters/interface.tex Outdated Show resolved Hide resolved
Copy link
Collaborator

@henrikt-ma henrikt-ma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting change because I think we can actually write the rules for variability analysis so that they properly describe how it needs to be done today.

To do this, I suggest that we add a section called Function Variability before Constant Expressions, where we explain how a variability is associated with every function class or component. Then we take this into consideration when defining the various expression variabilities.

chapters/operatorsandexpressions.tex Outdated Show resolved Hide resolved
@HansOlsson
Copy link
Collaborator Author

Is it getting closer now?

@henrikt-ma
Copy link
Collaborator

Is it getting closer now?

Yes, with minor details out of the way, we should now be able to focus on the principal question of how the variability of a function call should be defined.

chapters/interface.tex Show resolved Hide resolved
chapters/operatorsandexpressions.tex Outdated Show resolved Hide resolved
chapters/operatorsandexpressions.tex Outdated Show resolved Hide resolved
@henrikt-ma
Copy link
Collaborator

One thing that I didn't see discussed is how "impure" might affect variability.

I have also thought about that in relation to this PR. I wonder if 3.8 Variability of Expressions just wasn't updated when pure/impure was introduced, or if the intention of the rules in 12.3 Pure Modelica Functions is to keep impurity orthogonal to variability by setting up impure functions so that it isn't possible to sneak in a higher variability by the use of an impure function. Even if the latter is the case, I would appreciate if this was written out in 3.8, because to me it looks like an omission to say that f() has constant variability when f is impure.

It might be enough to just add a paragraph to 3.8 explaining why the variability of impure function calls doesn't matter, and then change all the rules for function call variability in the subsections to say "pure function". However, I guess this is actually something we could discuss in a separate PR, allowing the present PR to assume we're just talking about pure functions?

@HansOlsson HansOlsson added this to the Phone 2021-4 milestone Jun 12, 2021
HansOlsson and others added 2 commits June 23, 2021 15:26
Co-authored-by: Elena Shmoylova <eshmoylova@users.noreply.github.com>
chapters/functions.tex Outdated Show resolved Hide resolved
Copy link
Collaborator

@henrikt-ma henrikt-ma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're approaching something that could be good for now, but I think we can take the function variability a small step further to tie it even harder to function call variability:

  • Let's just define function variability as a variability associated with the function itself, and define this in the Function Variability section.
  • When defining variability of function call expressions, also take the function variability into consideration.

@HansOlsson
Copy link
Collaborator Author

I think we're approaching something that could be good for now, but I think we can take the function variability a small step further to tie it even harder to function call variability:

  • Let's just define function variability as a variability associated with the function itself, and define this in the Function Variability section.
  • When defining variability of function call expressions, also take the function variability into consideration.

I don't see this, as I don't see functions having variability; and thus points disappear.
To me this is related to why purity is ignored for variability, and I will explain it in that context.

Co-authored-by: Henrik Tidefelt <henrikt@wolfram.com>
@henrikt-ma
Copy link
Collaborator

I think we're approaching something that could be good for now, but I think we can take the function variability a small step further to tie it even harder to function call variability:

  • Let's just define function variability as a variability associated with the function itself, and define this in the Function Variability section.
  • When defining variability of function call expressions, also take the function variability into consideration.

I don't see this, as I don't see functions having variability; and thus points disappear.

Without anyone else showing interest in my way of attaching variability to the function itself, I have to give up this idea for now.

Copy link
Collaborator

@henrikt-ma henrikt-ma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to just remain some discussion about how to motivate why purity isn't considered for variability.

chapters/operatorsandexpressions.tex Outdated Show resolved Hide resolved
HansOlsson and others added 2 commits November 8, 2021 10:06
Co-authored-by: Henrik Tidefelt <henrikt@wolfram.com>
Co-authored-by: Henrik Tidefelt <henrikt@wolfram.com>
Copy link
Collaborator

@henrikt-ma henrikt-ma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Guessing that an updated review is wanted at this time.)

I think this PR is good enough now considering that the idea of attaching variability to the functions didn't attract a lot of positive attention.

@HansOlsson HansOlsson merged commit 57e159f into modelica:master Nov 10, 2021
@HansOlsson HansOlsson deleted the ClarifyFunctions branch November 10, 2021 09:09
@HansOlsson HansOlsson added the M36 For pull requests merged into Modelica 3.6 label Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
M36 For pull requests merged into Modelica 3.6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restriction of the default values of function inputs and outputs
6 participants