Skip to content
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

Set the surface area of individual nodes #112

Open
lrntct opened this issue Dec 4, 2017 · 8 comments
Open

Set the surface area of individual nodes #112

lrntct opened this issue Dec 4, 2017 · 8 comments

Comments

@lrntct
Copy link

lrntct commented Dec 4, 2017

The dynamic wave model needs that each node has a surface area. By default, this MinSurfArea is set to ~12 ft², which is equivalent to a 4ft diameter.
It is possible to set a global parameter to change this value.
However, network nodes could vastly vary in size, from a small 80cm manhole to a multi-metres pits in large mains. A value per node might be more accurate.

@lrntct lrntct changed the title Allow to set the surface area to individual nodes Set the surface area to individual nodes Dec 4, 2017
@dickinsonre
Copy link

If you want a value per node you should convert the default manholes or junctions to storage nodes and use a constant surface area for the storages.

@bemcdonnell
Copy link
Member

It depends how you would propose to implement this. If the API could access this value only, it would be interesting to discuss. On the other hand, a storage node is a very simple approach that wouldn’t change the internals in a huge way

@bemcdonnell
Copy link
Member

Side note; I’m growing more and more comfortable with having features available to the api user. In the future, we can re envision the structure of an input file which will allow us to have more detail.

@lrntct lrntct changed the title Set the surface area to individual nodes Set the surface area to individual junctions Dec 5, 2017
@lrntct
Copy link
Author

lrntct commented Dec 5, 2017

Of course, from a numerical point of view, every node is a storage. A junction is just a very simple one.
However, from the user standpoint, I believe that a storage is a structure specifically design to store water. If a user wants to list the storages of a model, they might not expect to receive a list including all the "custom" junctions.

My proposition would not be simply changing the API, because the node storage capacity changes the hydraulic behaviour. I was thinking of adding a surfaceArea value to TNode, and change node_getSurfArea() to return Node[j].surfaceArea as default. It is not a big internal change IMHO, but I'm not too familiar with the code base to see other implications. I can make a PR if you agree.

Side note; I’m growing more and more comfortable with having features available to the api user. In the future, we can re envision the structure of an input file which will allow us to have more detail.

I was more thinking of a functionality available through the API only. When the input API will be ready, it will be time to rethink the input file format. For now, it might be easier to keep the "legacy" input file access "legacy" functionalities only.

@lrntct lrntct changed the title Set the surface area to individual junctions Set the surface area of individual nodes Dec 5, 2017
@RudyFrom3
Copy link

The surface area of a node, is very often not the opening area that presents on the ground surface. The NODE is in fact a small storage where the internal depth and area define that storage. However the inflow characteristics are defined by a combination of EFFECTIVE weir length and EFFECTIVE area, or more simply by a rating curve of Depth Versus Flow. Possibly calling this the Internal Horizontal Area instead of Surface Area might be a good idea as it leads one to think it controls the quantity of flow able to enter the NODE ? See #111 for further comment

@lrntct
Copy link
Author

lrntct commented Dec 7, 2017

@RudyFrom3 This issue is about setting the manhole surfaceArea, not its opening. Those are two different matters. See my answer in #111.

@RudyFrom3
Copy link

Ok... so the next phase of this is to discover how and where in the code to add these capabilities I guess ??

@lrntct
Copy link
Author

lrntct commented Dec 8, 2017

Ok... so the next phase of this is to discover how and where in the code to add these capabilities I guess ??

I already have the code working. I just need to make sure to update the fullVolume when updating the surfaceArea. I hope to make a PR on Friday.

karosc pushed a commit that referenced this issue Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants