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

Node Lateral Inflow Bug #61

Closed
bemcdonnell opened this issue Jul 24, 2017 · 4 comments
Closed

Node Lateral Inflow Bug #61

bemcdonnell opened this issue Jul 24, 2017 · 4 comments
Assignees
Labels
Milestone

Comments

@bemcdonnell
Copy link
Member

@fmyers caught a small bug that is almost harmless.

In routing.c under routing_execute() there is a loop that sets the oldLatFlow to the newLatFlow for all the nodes:

    // --- initialize lateral inflows at nodes
    for (j = 0; j < Nobjects[NODE]; j++)
    {
        Node[j].oldLatFlow  = Node[j].newLatFlow;
        Node[j].newLatFlow  = 0.0;
    }

then all the lateral inflows are added to the nodes

Then it jumps down and loops through all the nodes again calling a function in nodes.c node_setOldHydState()

            for (j = 0; j < Nobjects[NODE]; j++)
            {
                node_setOldHydState(j);
                node_initInflow(j, routingStep);
            }

where node_setOldHydState() also moves OldLatFlow to NewLatFlow

void node_setOldHydState(int j)
//
//  Input:   j = node index
//  Output:  none
//  Purpose: replaces a node's old hydraulic state values with new ones.
//
{
    Node[j].oldDepth    = Node[j].newDepth;
    Node[j].oldLatFlow  = Node[j].newLatFlow; // <- Not Needed (according to Lew and Fred).
    Node[j].oldVolume   = Node[j].newVolume;
}
@bemcdonnell bemcdonnell added this to the v5.1.13 milestone Jul 24, 2017
@bemcdonnell bemcdonnell self-assigned this Jul 24, 2017
@dickinsonre
Copy link

@bemcdonnell I also added a link to https://www.openswmm.org/SWMM51012/routing-c in the interest of cross referencing SWMM5 issues. Is this okay?

@bemcdonnell
Copy link
Member Author

@dickinsonre sure! no problem!

@fmyers
Copy link

fmyers commented Jul 24, 2017

@dickinsonre The unneeded line of code is actually in node.c, within node_setOldHydState(). I only mention it because of your link to https://www.openswmm.org/SWMM51012/routing-c and I was wondering if there is one for node-c too.

The sequence of execution ended up performing these operations, though not all in the same routine:

  1. Node[j].oldLatFlow = Node[j].newLatFlow;
  2. Node[j].newLatFlow = 0.0;
  3. Node[j].oldLatFlow = Node[j].newLatFlow;

Line 3 ends up always forcing Node[j].oldLatFlow to 0.0 rather than leaving it with the lateral inflow from the prior routing step. Removing the line in node.c: node_setOldHydState() allows oldLatFlow to retain the prior newLatFlow value.

@dickinsonre
Copy link

Thanks, I am adding your further comments to the CHI website @fmyers

@bemcdonnell bemcdonnell modified the milestones: v5.1.13, v5.2.0.dev3 Mar 9, 2018
michaeltryby added a commit to SWMM-Project/swmm-solver that referenced this issue Sep 10, 2020
In addition to improvements described in ReleaseNotes this PR includes: 
 - Improvements to cmake build system
 - Testing scripts previously found in /tools have migrated to their own repo
 - Support for GitHub Actions has been added
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants