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

Issue147 movers record #153

Merged
merged 20 commits into from
Mar 6, 2015
Merged

Issue147 movers record #153

merged 20 commits into from
Mar 6, 2015

Conversation

mwetter
Copy link
Contributor

@mwetter mwetter commented Jan 25, 2015

Merge new pumps and fans.
This will close #147

@mwetter
Copy link
Contributor Author

mwetter commented Feb 27, 2015

@damienpicard : Can you please review. This is a big change that we want to have soon on the master as I have another major merge to do prepare soon that will affect the master and this branch.

@damienpicard
Copy link
Contributor

I will look at this tomorrow.

each min=0, each displayUnit="Pa")
"Fan or pump total pressure at these flow rates";

// This is needed for OpenModelica.
// fixme: Check if this can be put into FlowMachineInterface instead of here.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you want to keep some fixme in the master?

etaHyd = cha.efficiency(data=hydraulicEfficiency, r_V=r_V, d=hydDer);
etaMot = cha.efficiency(data=motorEfficiency, r_V=r_V, d=motDer);
etaHyd = cha.efficiency(per=per.hydraulicEfficiency, V_flow=VMachine_flow, d=hydDer, r_N=1, delta=1E-4);
etaMot = cha.efficiency(per=per.motorEfficiency, V_flow=VMachine_flow, d=motDer, r_N=1, delta=1E-4);
dpMachine = -dp;
VMachine_flow = -port_b.m_flow/rho_in;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is VMachine_flow = - port_b.m_flow/rho_in and not =port_a.m_flow/rho_in? rho_in is the density of inlet fluid...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will correct this. Using port_a.m_flow is better.

"Volume flow rate vs. total pressure rise with correction for pump resistance added";
// fixme check whether these branches are correct
// fixme check whether these branches are correct
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these fixme still need to be solved?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are no longer needed. I will remove them.

@damienpicard
Copy link
Contributor

For me, this branch can be merged after having addressed my few comments.

mwetter added a commit that referenced this pull request Mar 6, 2015
@mwetter mwetter merged commit 9c945a7 into master Mar 6, 2015
@mwetter mwetter deleted the issue147_moversRecord branch March 6, 2015 07:38
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.

Revise Movers package so that the models take data records.
2 participants