-
Notifications
You must be signed in to change notification settings - Fork 89
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
Calculation of parameters #591
Comments
Good morning @tlmerbecks, happy new year and thank you for the follow-up. I have also thought about something into this direction (but not as nice as you suggested). I really like it and I think I will implement it together with a couple of other changes:
It will look something like this: def default_residual(self, parameter, **kwargs):
return parameter.val - parameter.function(**kwargs) And the parameter settings like this: @staticmethod
def get_parameters():
return {
'eta_s': dc_cp(
min_val=0, max_val=1, num_eq=1,
derivative=self.eta_s_deriv,
residual=self.eta_s_resi,
function=self.calculate_eta_s,
),
'P': dc_cp(
min_val=0, max_val=1, num_eq=1,
derivative=self.energy_balance_deriv,
residual=self.default_residual,
function=self.calculate_P,
)
} The reason why one parameter would NOT use the default function can be of three reasons: First, the function might not actually be formulated in the way In this formulation it is very simple, i.e. just tespy/src/tespy/components/turbomachinery/compressor.py Lines 192 to 219 in 8d47499
tespy/src/tespy/components/turbomachinery/compressor.py Lines 240 to 262 in 8d47499
The second reason is when convergence stability (avoiding mathematical errors) is required, i.e. in tespy/src/tespy/components/heat_exchangers/base.py Lines 443 to 495 in 8d47499
The The third reason would be a parameter, which does not utilize a value like we are discussion here, but is just an expression (e.g. the Stodola law for turbine): tespy/src/tespy/components/turbomachinery/turbine.py Lines 249 to 277 in 8d47499
So the envisioned solution would be:
Maybe you can give me a quick feedback of what you think about the solution :). Thank you! |
Ciao @fwitte, Happy New Year to you too! My only concern with the approach outlined above is that the renaming of the parameters (e.g. Personally, I perfer renaming As for the new calculate function, I am undecided... part of me is thinking that the field should just be called Other than that, I fully agree with the solution outlined in the bullet points in you previous post. |
It certainly would, but these will come at some point no matter what you do, thus the change will be a new major version, 0.8.x. this will also be the case with component property units, I want to implement that soon! Same for the concept with the variable space reduction...
I will stick with them then.
Since func and deriv do not use verbs, I would then go with definition I think. Thank you, have a nice day |
Ciao Francesco, thanks for merging my pull request for #587!
I have been thinking about this issue a bit further, and have gotten the feeling that such errors could in future be avoided by devolving the calculation of parameters from the residual and post-processing calculation.
For example, in this case, the error arose because the calculation of ttd_min had been replicated in various places and a typo had crept into one of these instances. Instead, we could define a function that calculates the parameter, which can then be called whenever needed, i.e. for the residual and post-processing calculations.
This could be implemented by adding another attribute to the parameter data container, namely a function to calculate the parameter, which is then called by the residual function. In the example below, "resi" (currently "func") is the function to calculate the residual and "func" is the function to calculate the parameter.
This could potentially also streamline the post-processing calculations, by allowing all properties to be calculated by looping over the parameters and calling the respective "func" function.
Hope this makes sense
The text was updated successfully, but these errors were encountered: