QSD-Group / QSDsan

Quantitative Sustainable Design (QSD) of sanitation and resource recovery systems.
https://qsdsan.com
Other
30 stars 14 forks source link

Do not add attributes that are constant (e.g., MW) #63

Closed yalinli2 closed 2 years ago

yalinli2 commented 2 years ago

In StruvitePrecipitation (won't b surprising if there are similar situations in other units), there are many attributes that are constant, it'll be better to just hard code those since they won't change, e.g., we know the molecular weight of P, there's no need for an attribute like MW_P (but it'll be good to add a note in the code about what that 30.97 means) https://github.com/QSD-Group/QSDsan/blob/e9fdbf1e6ed25685ce31093f5a58eced6c5eabc7/qsdsan/sanunits/_struvite_precipitation.py#L98

However, for attributes such as P_rec_1, it still might be better to add them as attributes, as they could change

On this end, why the attribute is called P_rec_1? What does that 1 stand for? Similarly for K_rec_1. Without a reason, it's confusing

Additionally, if want the mol, use imol instead of imass

yalinli2 commented 2 years ago

P_rec_1, P_rec_2, used to be needed to differentiate between recoveries

lsrowles commented 2 years ago

As noted, P_rec_1, P_rec_2 were used to be needed to differentiate between recoveries in Lohman et al. 2020 where 1 represented assumptions related to struvite precipitation and was for assumptions related to ion exchange.

yalinli2 commented 2 years ago

Sounds great, I've updated the codes to remove the constant attributes and added comments at appropriate places.

I also removed the _1, _2, etc. suffixes - this shouldn't be confusing since these recoveries are used in different units, I added the old names like P_rec_1 to the "name (if diff)" column in the data sheets, that column is intended for this purpose (to note the new names for an attribute if they are different from the reference paper)