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

Clock constructor dimensionality. #2768

Merged
merged 6 commits into from Dec 18, 2020
Merged

Conversation

HansOlsson
Copy link
Collaborator

Closes #2177

chapters/synchronous.tex Outdated Show resolved Hide resolved
chapters/synchronous.tex 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, since this doesn't sounds exactly like what I think was said in the meeting.

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

I have now dropped 'in a similar way'; I couldn't figure out how to easily state that there are no clocks in function - so I think it can wait.

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, since I don't think the intention was to allow operators to take arrays of expressions.

chapters/synchronous.tex Outdated Show resolved Hide resolved
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.

Well, at least this now meets my expectations based on my understanding of the phone meeting decision, so I leave my approval.

As my mentions of @sjoelund hasn't triggered a response from him yet, I now made a last attempt to also get his approval by requesting his review. Maybe we can wait a day before merging?

Copy link
Member

@sjoelund sjoelund 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 the comment from @eshmoylova makes sense. Perhaps "These functions are not callable in functions and functions may not contain Clock variables".
I don't have time to look at this in detail though.

@henrikt-ma
Copy link
Collaborator

henrikt-ma commented Dec 18, 2020

I think the comment from @eshmoylova makes sense. Perhaps "These functions are not callable in functions and functions may not contain Clock variables".
I don't have time to look at this in detail though.

It's a valid restriction to make, but not in this section. Right after the definition of Clock variable, for instance, would be a possible place to add it.

@HansOlsson
Copy link
Collaborator Author

I think the comment from @eshmoylova makes sense. Perhaps "These functions are not callable in functions and functions may not contain Clock variables".
I don't have time to look at this in detail though.

It's a valid restriction to make, but not in this section. Right after the definition of Clock variable, for instance, would be a possible place to add it.

Or in function chapter as suggested by @eshmoylova - I added 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.

Looks good, and I think it's ready to be merged now.

@HansOlsson HansOlsson merged commit b8150d2 into modelica:master Dec 18, 2020
@HansOlsson HansOlsson deleted the ClockDim branch December 18, 2020 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve specification of clock constructors with respect to dimensionality
4 participants