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

Added comments #93

Merged
merged 4 commits into from May 9, 2014

Conversation

Projects
None yet
3 participants
@tbeu
Contributor

tbeu commented Apr 14, 2014

If you like it.

I also though about Units, e.g. there is Modelica.SIunits.TimeAging for alpha and gamma, but no specific TimeAging for beta and delta.

Added comments
If you like it.

I also though about Units, e.g. there is Modelica.SIunits.TimeAging for alpha and gamma, but no specific TimeAging for beta and delta.
@xogeny

This comment has been minimized.

Owner

xogeny commented Apr 14, 2014

Thomas,

I think the strings are an improvement. The only problem is that you would need to add them to all the examples in this section. It would seem odd if they just disappeared as the section progressed.

Units are definitely not something I want to introduce here. The reason is that we haven't gotten to the point in the book yet where we talk about the Modelica Standard Library. So for now, we should stick to Reals. If you want to go through and make this change to all the other models in this package, then I would accept the pull request.

Thanks.

@tbeu

This comment has been minimized.

Contributor

tbeu commented Apr 15, 2014

Hopefully I caught them all.

Tested with Dymola and SimulationX.

@mtiller

This comment has been minimized.

Collaborator

mtiller commented Apr 16, 2014

This looks good. But I am going to hold off on merging until I can do a proper build of the book and make sure everything works. I'll try to process this by Friday.

@tbeu

This comment has been minimized.

Contributor

tbeu commented Apr 16, 2014

Take your time. No need to hurry. Happy Easter!

xogeny added a commit that referenced this pull request May 9, 2014

@xogeny xogeny merged commit e7ab551 into xogeny:master May 9, 2014

@tbeu

This comment has been minimized.

Contributor

tbeu commented May 9, 2014

@mtiller e7ab551 removed the start values from ClassicModelInitialEquations. This should be undone. See my comments above and check again.

@tbeu tbeu deleted the tbeu:patch-3 branch May 9, 2014

@xogeny

This comment has been minimized.

Owner

xogeny commented May 9, 2014

How is this: a469f42

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