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

Loading outfalls crashes depending on order #187

Open
MortenGrum opened this issue Mar 21, 2023 · 4 comments
Open

Loading outfalls crashes depending on order #187

MortenGrum opened this issue Mar 21, 2023 · 4 comments
Assignees
Labels

Comments

@MortenGrum
Copy link

Loading outfalls seems to crash depending on the order of the free and timeseries staged outfalls. Perhaps a bug?

Loading these outfalls works fine:`

[OUTFALLS]
;;Name           Elevation  Type       Stage Data       Gated    Route To        
;;-------------- ---------- ---------- ---------------- -------- ----------------
StagedOutfall    0          TIMESERIES OutfallStage     NO                       
FreeOutfall      0          FREE                        NO                       

but these outfalls raise an IndexError:

[OUTFALLS]
;;Name           Elevation  Type       Stage Data       Gated    Route To        
;;-------------- ---------- ---------- ---------------- -------- ----------------
FreeOutfall      0          FREE                        NO                       
StagedOutfall    0          TIMESERIES OutfallStage     NO                       

The error is:

IndexError: failed to parse [OUTFALLS] with cols: ['Name', 'InvertElev', 'OutfallType', 'StageOrTimeseries']. head:
[OUTFALLS]


FreeOutfall      0          FREE                        NO
StagedOutfall    0          TIMESERIES OutfallStage     NO

This unittest fails for the inp file where the free outlet is first:

import os
import unittest
import swmmio

class SwmmioTest(unittest.TestCase):

    def test_swmmio_outfall(self):
        
        swmm_file = os.path.join('path to test files', 'outfalls_issue.inp')
        model = swmmio.Model(swmm_file)

        try:        
            outfalls = model.inp.outfalls
        except:
            self.assertTrue(False, 'Could not read outfalls from inp file')

        swmm_file_free_first = os.path.join('path to test files', 'outfalls_issue_free_first.inp')
        model_free_first = swmmio.Model(swmm_file_free_first)

        try:        
            outfalls_free_first = model_free_first.inp.outfalls
        except:
            self.assertTrue(False, 'Could not read outfalls from inp file when free outfall was first')

The two inp file both run in SWMM5.1 (.txt needs to be removed):
outfalls_issue.inp.txt
outfalls_issue_free_first.inp.txt

I am on Windows 11, python 3.9.7, pandas 1.5.1 and using swmmio version 0.6.2.

@aerispaha
Copy link
Member

@MortenGrum thank you for identifying this issue with so much detail! I'm going to investigate this over the next day or two. Hopefully we can leverage your unit test to fix this.

@aerispaha aerispaha self-assigned this May 15, 2023
@aerispaha aerispaha added the bug label May 15, 2023
BuczynskiRafal added a commit to BuczynskiRafal/swmmio that referenced this issue Jun 11, 2023
@lmontest
Copy link

lmontest commented Nov 22, 2023

Hey @aerispaha, this is the same issue I mentioned to you some time ago. Are you planning on merging this fix? Thank you @BuczynskiRafal. This addresses a bunch of issues that happens when data blocks have rows with more elements that the first row.

@aerispaha
Copy link
Member

Hi @lmontest! I would like to merge this fix. I'm having trouble constructing the PR though. This commit on @BuczynskiRafal 's fork seems to be in a unusual state, with GitHub showing this warning:

Warning

This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

@BuczynskiRafal can you advise? If you can create a PR with these changes, I'll review it asap.

@BuczynskiRafal
Copy link
Collaborator

BuczynskiRafal commented Nov 23, 2023

I think this warning came up because I closed PR. I closed PR because @aerispaha wrote that he was already working on a more general solution to this issue.
I can restore the PR but I'm afraid it didn't solve the problem completely because the whole test suite didn't pass.

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

4 participants