-
Notifications
You must be signed in to change notification settings - Fork 0
add stratified heat buffer #225
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
base: main
Are you sure you want to change the base?
Conversation
| self.timestep = 3600 # seconds | ||
| self.solver_asset = ProductionAsset(name=self.name, _id=self.asset_id) | ||
|
|
||
| # Output list |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove redundant comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| ) | ||
|
|
||
| def _calculate_fill_level(self) -> None: | ||
| """Calculate fill level of the storage.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also now the maximum volume, so should we not also calculate the volume stored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indeed we can have both output
| raise ValueError( | ||
| f"The new fill level is {new_fill_level}. It should be between 0 and 1." | ||
| ) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You only calculate the volume stored, but you should also track the temperature or the energy stored in the buffer, since now you assume that you can always supply at the temperature which is set, but you will supply at the temperature which is in the buffer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently I implement ideal buffer, only tracking volume without modeling temperature losses. Thus temperature of storage always at setpoint supply and return
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But still without temperature loss, you can have different temperatures as inflow and then you just want to calculate mixing temp of fluid in buffer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The above image is not what is being modelled based on the provided code. You have a storage with a 2-node component now.
In addition, you mention hot and cold remain constant but that is not always the case right? These temperatures can vary or are you now implying that the whole simulation has a fixed supply and return temperature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we conclude that for simplicity , the ideal buffer is tracking energy with a fixed deltaT. Since Energy = Power * dt and Power = deltaT * Flow * HeatCapacity. Thus tracking energy is similar with tracking volume V = V + Flow * dt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samvanderzwan @vanmeerkerk but you are correct, now the implementation is driven by temperature supplied by controller, it should be using temperature from the object itself defined from ESDL carrier.
let's discuss first if this model is enough for our simulator or we want to have more detailed model
vanmeerkerk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Ryvo I tried to understand the component but could not yet figure it out. Could you write a manual page describing the equations that you would like to solve?
| from omotes_simulator_core.entities.assets.asset_defaults import HEAT_BUFFER_DEFAULTS | ||
|
|
||
|
|
||
| class EsdlAssetAtesMapper(EsdlMapperAbstract): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is a mapper to a heatbuffer class why do I see the Ates referecne? Or do we no assume the Ates is the idealized heat buffer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i copy paste from ates, indeed i should change to buffermapper
| self.thermal_power_allocation = 0 | ||
| self.mass_flowrate = 0 | ||
| self.maximum_volume = maximum_volume | ||
| self.fill_level = fill_level |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I would rename fill_level to percentual_fill_level to make it more clear that we are using it as a fraction (e.g., value between 0 - 1)
| self.output: list = [] | ||
|
|
||
| def set_setpoints(self, setpoints: Dict) -> None: | ||
| """Placeholder to set the setpoints of an asset prior to a simulation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is no longer a placeholder, right?
| else: | ||
| raise ValueError( | ||
| f"The new fill level is {new_fill_level}. It should be between 0 and 1." | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This error should be handeld by the controller; a fill level of 0 means no discharge from the storage and 1 no more supply to the storage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still implementing this error to make sure the flow that you set from controller does not hit buffer volume constraint. It could be the controller is not taken account this
| self.maximum_volume = maximum_volume | ||
| self.fill_level = fill_level | ||
| self.current_volume = fill_level * maximum_volume | ||
| self.timestep = 3600 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Timestep should not be a fixed value but depend on the simulation dt; now it cannot be adjusted as the input is not there!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is similar with ates discussion, currently it is also fixed there until we have solution to include timestep from controller or esdl
| raise ValueError( | ||
| f"The new fill level is {new_fill_level}. It should be between 0 and 1." | ||
| ) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The above image is not what is being modelled based on the provided code. You have a storage with a 2-node component now.
In addition, you mention hot and cold remain constant but that is not always the case right? These temperatures can vary or are you now implying that the whole simulation has a fixed supply and return temperature.
| :param str asset_id: The unique identifier of the asset. | ||
| """ | ||
| super().__init__(asset_name=asset_name, asset_id=asset_id, connected_ports=port_ids) | ||
| self.temperature_supply = DEFAULT_TEMPERATURE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Temperature_supply is the 'hot storage temperature' and Temperature_return is the 'cold storage temperature'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes indeed, i can rename that also
| PROPERTY_TEMPERATURE_SUPPLY, | ||
| PROPERTY_TEMPERATURE_RETURN, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The controller is not able to set these values at least not with my understanding of the current implementation. I do not see how it can continously change the hot and cold storage temperatues. You should get some sort of value for the energy it is supplying to the vessel and calculate the new fluid level associated with the mixed hot/cold temperature level, right?
…rom carrier esdl during mapping
…rom carrier esdl during mapping
…ct-OMOTES/simulator-core into 221-day-night-buffer-asset # Conflicts: # src/omotes_simulator_core/entities/assets/asset_defaults.py
|
i did major re-implementation of water buffer tank. based on 1-D stratified buffer with 5 layers |
reimplement as 1D stratified buffer tank
|
reference: https://www.sciencedirect.com/science/article/pii/S0306261919307901?via%3Dihub but i dont include buoyancy and thermal losses to outside, only direct heating (yellow part)
|
vanmeerkerk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor comments should be fine after a final fix
| cd /D "%~dp0" | ||
| cd ..\..\ | ||
| flake8 .\src\omotes_simulator_core | ||
| flake8 .\src\omotes_simulator_core .\unit_test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you either remove this statement or explain why it is needed to also run flake8 on the unit test separately?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because the linux script also include unit test, so i just want to make sure it is similar
| call .\venv\Scripts\activate | ||
| set PYTHONPATH=.\src\;%$PYTHONPATH% | ||
| python -m mypy ./src/ | ||
| python -m mypy ./src/omotes_simulator_core ./unit_test/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as with the linting. Why is it necessary also call mypy on the unit test? Shouldn't the pyproject.toml handle this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because the linux script also include unit test, so i just want to make sure it is similar
| asset_name=esdl_asset.esdl_asset.name, | ||
| asset_id=esdl_asset.esdl_asset.id, | ||
| port_ids=esdl_asset.get_port_ids(), | ||
| volume=esdl_asset.get_property( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You won't need to call the EsdlAssetObject.get_property()[0] with the zero at the end; the method returns a float so it should work without.
| # TODO: The loop is not complete as the asset also has a fill-level that should not | ||
| # surpass the maximum fill-level. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is an implementation I made that can be merged/changed after we have completed the current PR.
| # You should have received a copy of the GNU General Public License | ||
| # along with this program. If not, see <https://www.gnu.org/licenses/>. | ||
|
|
||
| """Test Ates Cluster entities.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename to test the HeatBuffer or descriptive name of the asset you are testing.
|
|
||
| def _set_solver_asset_setpoint(self) -> None: | ||
| """Set the setpoint of solver asset.""" | ||
| if self.mass_flowrate > 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be the zero flow limit; >= MASSFLOW_ZERO_LIMIT and probably the lowerbound should be <= - MASSFLOW_ZERO_LIMIT. Question, what happens when m_dot = 0 or between those bounds?
| self.mass_flowrate = 0 | ||
|
|
||
| def _calculate_new_temperature(self) -> None: | ||
| """Calculate new temperature of the tank storage.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you include more details in the docstring what you are exactly solving?
|
|
||
|
|
||
| class HeatBuffer(AssetAbstract): | ||
| """A HeatBuffer represents an asset that stores heat. Thus, it has the possibility to supply \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe include a reference or doi to the paper you are using to model this system.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The temperature at the in and out port should be possible to define right? Or at least some sort of initial storage temperature, now you will always start at 300 k
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The layer mass now remains constant over the calculation; shouldn't that be a parameter defined by the local temperature difference over the layer? So that layer mass is a variable over the height of the storage tank?
This off course depends on the structure of the tank
| # TODO: Current implementation loops over the entire profile; should be improved! | ||
| # TODO: Unclear why there is a timestep of 1 hour in the profile. | ||
| for index in range(self.start_index, len(self.profile)): | ||
| if abs((self.profile["date"][index].to_pydatetime() - time).total_seconds()) < 3600: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might not work as expected; what is the intention? Maybe we should make an seperate issue to review this part.
| """Method to get the total storage discharge power of the network. | ||
| :return float: Total heat discharge of all storages. | ||
| """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still something that needs to be implemented. Revert back the todo.
|
I checked some more of the implementation, when I create a system with one layer the stored power is approximately 36 MJ after one step (1H), but this increases to 97 MJ for 5 layers, even though the input doesn't change. There might be something wrong with the implementation @octavianor --- Addition --- I tried a few things - please correct me if I am wrong - but do you now compute the stored power as: ` ` |
|
i put this into draft because i confuse with convension positive and negative flowrate |
|
implement ODE solver, has been tested with esdl file |
|
@vanmeerkerk ODE takes 30 iteration for 5 layers (with solving time 1 ms) |



now we implement not ideal buffer, it means that the temperature of return and supply temperature depends on the states of the storage.
this makes difficult for controller to do balancing based on power. Example: controller asking 1 MW from storage. Storage will calculate mass flowrate based on dT. The consequence: network temperature becoming 75 degree at supply side, and it will go down over time when you ask more power from storage.
in Warming Up 1, we assumed ideal buffer with fixed dT so we have no problem