-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Network intervention simulation #345
Conversation
You've used my proposal for adding network path capabilities to QWAT. The pipe_reference table should be explained a little bit more, it needed geometry because otherwise we couldn't have a real path shown. The pipe_reference table must be kept up to date because otherwise the path could be different from what the user sees. |
@benoitblanc Does this has a connection with what kind of network simulation software? Does this comply with the SIA405 water datamodel (Hydraulischer_Strang, hydraulischer_Knoten)? |
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.
Hi,
I have been asked to perform a review.
It's really nice to see these features getting into the code base, they have been long awaited!
It's a bit hard to jump quickly into the code and I think we miss a few things generally:
- a couple of lines explaining how this work, directly in the code: the roles of the tables and functions
- documenting each SQL object a bit more in detail, it would help a lot understanding the code
- tests are missing: I believe you had some data to play with, this should be added to the code base for such functionality
- for now there are only deltas, the code should also be added in the source (but I guess you plan to do it only at the end)
Some questions:
- wouldn't there be a way to avoid code duplication for the routing method? maybe by creating lower level functions which could be reused?
- it's not clear to me the reason of
vw_pipe_reference
, is it because the table behind might not be up to date? - instead of using tables and functions, why not using materialised views?
update/delta/delta_1.3.7_002_search_network_and_subscribers.sql
Outdated
Show resolved
Hide resolved
update/delta/delta_1.3.7_002_search_network_and_subscribers.sql
Outdated
Show resolved
Hide resolved
update/delta/delta_1.3.7_002_search_network_and_subscribers.sql
Outdated
Show resolved
Hide resolved
does it fix #171 ? |
Since there are only deltas, the network simulation are not initialized when initializing from scratch : https://github.com/qwat/qwat-data-model/blob/master/init_qwat.sh Please add the necessary files (same content as the deltas) and code adaptation to make it work for new users of TEKSI water module. |
@ponceta I have added code from deltas to data-model and |
Something doesn't please the CI. Is it possible that the delta initialize the functions but does not run these and therefore does not create the tables or vice versa? Check...DIFFERENCES FOUND
|
@benoitblanc could you rename the deltas to 1.4.0_XXX |
…e able to go through all pipes, nodes and valves
c9c6078
to
5f7a1f7
Compare
@ponceta I have renamed deltas to match v 1.4.0 |
This comment was marked as resolved.
This comment was marked as resolved.
@benoitblanc if you're ready to merge, please remove the [WIP] Tag in the name of your pull request. |
This pull request aims to add some features for the user to simulate network incidents :