search

IDAES / idaes-pse

The IDAES Process Systems Engineering Framework
https://idaes-pse.readthedocs.io/
Other
214 stars 231 forks source link

Scaling missing for some control volume terms #175

Closed TimBartholomew closed 3 years ago

TimBartholomew commented 4 years ago

The control volume doesn't automatically scale the additional variables associated with has_mass_transfer and has_enthalpy_transfer.

We probably should go through and double check all the options we provide to control volumes and also provide tests for them.

TimBartholomew commented 4 years ago

@andrewlee94 since you moved it to "in progress", are you starting to address this?

Another thing I've noticed for my RO model and other unit models that have an energy balance, I get the following warning: fs.P1.control_volume.properties_in[0.0].flow_mass_comp[H2O] + fs.P1.control_volume.properties_in[0.0].flow_mass_comp[NaCl])*fs.P1.control_volume.properties_in[0.0].enth_mass

So it may just be an iscale.get_scaling_factor() on the enthalpy term, but I'm not sure.

andrewlee94 commented 4 years ago

No, I am not. However, I put it there so that it is part of the September release.

andrewlee94 commented 4 years ago

Also, what is the actually error/warning you got?

TimBartholomew commented 4 years ago

Oops. Its:

[WARNING] idaes.core.util.scaling: Accessing missing scaling factor for (fs.P1.control_volume.properties_in[0.0].flow_mass_comp[H2O] + fs.P1.control_volume.properties_in[0.0].flow_mass_comp[NaCl])*fs.P1.control_volume.properties_in[0.0].enth_mass

eslickj commented 4 years ago

Weird. That would usually be a variable name but it's showing an expression. Seems like we are trying to pull a scale factor for an unnamed expression.

andrewlee94 commented 4 years ago

Given the amount of flexibility we have, I don't think we can make any assumptions about whether an object is a Var or Expression (except for maybe a few key cases).

eslickj commented 4 years ago

Yeah. I guess it doesn't cause any problem. You can provide scaling factors for named expressions, but there is no way to store a scaling factor for an unnamed one. In fact it's weird that we have a pointer to an unnamed expression accessible.

eslickj commented 4 years ago

Ok, I think I know what this is. See issue https://github.com/IDAES/idaes-dev/issues/1068. I closed it so I must have thought I caught all the property packages but I guess not. I'm 80% sure this is the problem (without actually looking into it).

andrewlee94 commented 3 years ago

Fixed in #55.