Skip to content

Conversation

@AbelHeinsbroek
Copy link
Contributor

See issue #885 for more information

@LRossman
Copy link
Collaborator

I have a few comments.

  1. I don't see the need to introduce the hydraul.IsIllConditioned variable, since it's enough to check if hydraul.ErrNode is > 0. Plus Error 110 already indicates that hydraulics couldn't be solved.
  2. Maybe I missed it, but I don't see where ErrNode is initialized hydsolver.c.
  3. You've introduced a new error message, 265,"network is not ill-conditioned", which does not really indicate an error condition like all of the other error codes do. I suggest you remove it.

@AbelHeinsbroek
Copy link
Contributor Author

Thank you for the feedback, I've made the suggested changes. I removed the new error message, and use 203 - "undefined node" instead.

@LRossman
Copy link
Collaborator

I don't think you want to use 203 as an error for when ErrNode = 0. ErrNode = 0 is a valid result, indicating that there is no node causing the network to be ill-conditioned. So please remove lines 1103 - 1105 in the latest commit.

Also, although you've now initialized ErrNode in hydsolver.c, I wonder if all of the variables retrieved with the EN_getstatistic function need to be initialized when a network is first created, since the function could be called before any analysis has been made in which case the variables would contain garbage. This is probably best left as a new PR.

@AbelHeinsbroek
Copy link
Contributor Author

right, I forgot that nodes and links are not zero-indexed. I've made the changes.

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

Successfully merging this pull request may close these issues.

2 participants