-
Notifications
You must be signed in to change notification settings - Fork 1
implement NVOE calculation to avoid sand production #329
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
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.
Pull request overview
This PR implements NVOE (Net Volume Over Extraction) calculations to prevent sand production in ATES (Aquifer Thermal Energy Storage) systems by dynamically limiting charge and discharge flow rates based on physical constraints.
Key Changes:
- Added methods to calculate maximum safe flow rates for injection and extraction based on aquifer properties, temperature, and pressure
- Implemented dynamic calculation of saline density and viscosity to improve accuracy
- Added state management to propagate calculated power limits to the controller
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
controller_storage.py |
Added set_state method to update maximum charge/discharge power limits from ATES calculations |
ates_cluster.py |
Implemented NVOE calculations with helper methods for saline properties, maximum flow rates, and state propagation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rosim_input__flow = [volume_flow, -1 * volume_flow] # first elemnt is for producer well | ||
| # and second element is for injection well, positive flow is going upward and negative flow | ||
| # is downward | ||
| rosim_input_flow = [volume_flow, -1 * volume_flow] # the first-element is for hot well |
Copilot
AI
Nov 25, 2025
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.
Corrected 'first-element' to 'first element' (remove hyphen).
| rosim_input_flow = [volume_flow, -1 * volume_flow] # the first-element is for hot well | |
| rosim_input_flow = [volume_flow, -1 * volume_flow] # the first element is for hot well |
| rosim_input_flow = [volume_flow, -1 * volume_flow] # the first-element is for hot well | ||
| # and the second-element is for cold well. positive flow is charge and negative flow |
Copilot
AI
Nov 25, 2025
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.
Corrected 'second-element' to 'second element' (remove hyphen).
| rosim_input_flow = [volume_flow, -1 * volume_flow] # the first-element is for hot well | |
| # and the second-element is for cold well. positive flow is charge and negative flow | |
| rosim_input_flow = [volume_flow, -1 * volume_flow] # the first element is for hot well | |
| # and the second element is for cold well. positive flow is charge and negative flow |
|
|
||
| def get_state(self) -> dict[str, float]: | ||
| """Function to calculate the maximum charge and discharge rate based on NVOE.""" | ||
| P = self.aquifer_depth * 0.1 # bar assume pressure increase 1 bar per 10 m depth |
Copilot
AI
Nov 25, 2025
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 comment is missing punctuation and proper capitalization. Consider: '# Bar - assume pressure increases 1 bar per 10 m depth'.
| P = self.aquifer_depth * 0.1 # bar assume pressure increase 1 bar per 10 m depth | |
| P = self.aquifer_depth * 0.1 # Bar - assume pressure increases 1 bar per 10 m depth |
| max_flowrate = max_flowrate * self.aquifer_depth * 0.01 # using depth factor because ATES | ||
| # is deeper than WKO |
Copilot
AI
Nov 25, 2025
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 magic number 0.01 lacks explanation. Consider defining it as a named constant (e.g., DEPTH_CORRECTION_FACTOR = 0.01) with a comment explaining its physical meaning and why it's specific to ATES vs WKO.
samvanderzwan
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.
See the comments.
In general avoid magic numbers and ensure the documentation of methods is up to date explaining what a method does.
| self.salinity = salinity # ppm | ||
| self.well_casing_size = well_casing_size # inch | ||
| self.well_distance = well_distance # meters | ||
| self.wellbore_diameter = 31.0 # inch |
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.
These are magic numbers, and need to be either defined in the asset defaults and better yet have them is input argument and set them in the mapper.
Next to this you have nicely added the unit, but you are now using in the same assets inch and m, this will lead to errors in future, please only use SI. When you need to pass non SI units to for example Rossim do the conversion at the latest possible moment
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 will add in asset default since this value is not defined in ESDL
| SALINITY = self.salinity | ||
| WELL2_X = self.well_distance + 300 | ||
| CASING_SIZE = self.well_casing_size | ||
| CASING_SIZE = self.wellbore_diameter |
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 does not seem logical, you would expect here the casing size
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 now change to wellbore diameter
| """Function to calculate storage temperature after injection and production.""" | ||
| volume_flow = self.mass_flowrate * 3600 / 1027 # convert to second and hardcoded saline | ||
| # density needs to change with PVT calculation | ||
| saline_density = self._get_saline_density( |
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.
What is the 20?
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.
forgot to replace the pressure here based on depth
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.
Oke, but the comment is now incorrect, since it states per 10 m. Next to this it also assumes density of 1000 and g of 10, better to use actual values, since you have them.
| self.cold_well_temperature = celcius_to_kelvin(ates_temperature[1]) # convert to K | ||
|
|
||
| def get_state(self) -> dict[str, float]: | ||
| """Function to calculate the maximum charge and discharge rate based on NVOE.""" |
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 the get state method, to get the state of the object at a certain time step.
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 using this get state method to calculate the new maximum flowrate, do you have any other method?
| # diameter | ||
| well_radius = 0.5 * self.wellbore_diameter * 0.0254 # m | ||
|
|
||
| cloggingVel = 0.3 |
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 not clear where this number come from, and it is not good practice to have this kind of numbers somewhere in a method.
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 put reference parameters value
|
|
||
| max_flowrate = ( | ||
| 2 * math.pi * well_radius * self.aquifer_thickness * max_infiltrate_flow_velocity | ||
| ) # m/h |
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.
should be m3/h
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.
true
| return max_flowrate | ||
|
|
||
| def _get_saline_density(self, P: float, T: float) -> float: | ||
| """Function to calculate the saline density.""" |
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.
what are the inputs, what is the return value
What approach do you use?
This method needs more explanation, since there are some formulas in but it is not clear where they come from. It can also be added to the rst.
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 have attach the reference and will add to rst
|
|
||
| def _get_saline_density(self, P: float, T: float) -> float: | ||
| """Function to calculate the saline density.""" | ||
| P = P * 1e5 * 1e-6 # Bar to MPa |
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 works, since P is a float and P outside of the method is not changed. but better to give it a different name.
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 will change
| return density | ||
|
|
||
| def _get_saline_viscosity(self, P: float, T: float) -> float: | ||
| """Function to calculate the saline viscosity.""" |
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.
See comments previous method
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.
is its using Batzle-Wang correlation
samvanderzwan
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.
There are still some comments open from previous review.
Other changes are ok
| """Function to calculate storage temperature after injection and production.""" | ||
| volume_flow = self.mass_flowrate * 3600 / 1027 # convert to second and hardcoded saline | ||
| # density needs to change with PVT calculation | ||
| saline_density = self._get_saline_density( |
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.
Oke, but the comment is now incorrect, since it states per 10 m. Next to this it also assumes density of 1000 and g of 10, better to use actual values, since you have them.
| water_density = fluid_props.get_density(average_temperature) | ||
| water_heat_capacity = fluid_props.get_heat_capacity(average_temperature) | ||
|
|
||
| max_extraction_flow_cold_well = self._get_max_flowrate_extraction_norm( |
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.
still open
|
|
||
| def _get_max_flowrate_extraction_norm(self, P: float, T: float) -> float: | ||
| """Function to calculate the maximum flowrate of production in norm.""" | ||
| grav_accel = 9.81 # m/s2 |
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.
still open
| grav_accel = 9.81 # m/s2 | ||
| saline_density = self._get_saline_density(P, T) | ||
| saline_viscosity = self._get_saline_viscosity(P, T) | ||
| aquifer_permeability = self.aquifer_permeability * 9.8692326671601e-16 # mD to m2 |
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.
still open
No description provided.