search

FLO-2DSoftware / qgis-flo-2d-plugin

A plugin for pre-processing/post-processing FLO-2D models
8 stars 8 forks source link

SD Widget Editor: 'Add inlet type 4 data' :2 options: RT (currently the default) but also a Culvert Equation. #1092

Closed FLO-2DJJ closed 6 months ago

FLO-2DJJ commented 1 year ago 
          In the SD Widget Editor the option to 'Add inlet type 4 data' should consider 2 options: rating table (currently the default) but also a Culvert Equation.

image

Originally posted by @FLO-2DJJ in https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1052#issuecomment-1834065981

FLO-2DJJ commented 12 months ago

Already as pull request https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/pull/1102 to be reviewed

FLO-2DKaren commented 11 months ago

@FLO-2DJJ

You can push your branch for testing without building a pull request. That way I can test your branch without having to do review on the pull request. I will delete the pull request and you can keep working on your branch.

I will also copy my review comments here.

FLO-2DJJ commented 11 months ago

@FLO-2DKaren, @FLO-2DNoemi, @rpachaly The current remote master is wrong for several SD operations. I did something from Eclipse that rewinded the SD to old commits. My local changes for SD are right but can't push them to the remote. I'm working on this problem right now.

FLO-2DJJ commented 11 months ago

@FLO-2DKaren, @FLO-2DNoemi, @rpachaly Fixed. The commits/merge problem around SD is solved. The remote master is updated with the latest SD functionality:

FLO-2DKaren commented 11 months ago

@FLO-2DJJ OK I can test it now.

FLO-2DKaren commented 8 months ago

I found some new issues. I would also like to test this with one of Noemi's Naple models. I think she has one with loads of culvert data.

  1. When you add a new culvert, it isn't assigning the grid element number.
  2. There are 3 variables in the table that are being input as real numbers that should be integers.
  3. When you use the import or folder button, it is only looking for rating tables. Did I do something wrong here?

Here's a video to show my process.

https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/assets/20424460/02c281b5-5d17-4158-b74c-0b195129c6e6

FLO-2DKaren commented 8 months ago

@FLO-2DJJ Tested and found issues.

FLO-2DJJ commented 8 months ago

@FLO-2DKaren

I'll look at the issues 1) and 2) that you mentioned.

As for issue number 3) Import button (Add predefined... (folder icon)), the plugin looks for files with the same name of the inlet when a rating table is to be assigned to that inlet. But for Culvert equations the data should be in a predefined file named Type4Culvert.txt or Type4Culvert.dat (upper or lower cases is the same). Each line of Type4Culvert.txt defines the culvert equation for one inlet:

grid inlet cdiameter typec typeen cubase multbarrels

For instance,

imagen

FLO-2DKaren commented 8 months ago

OK good. But we won't know the grid number in this file. This sort of data will come from an hy8 script or excel. It will be ignorant of grid location.

Please eliminate the grid number. That should be set by the schema data or the user data.

Can you add a header? It can start with a ";" or something to identify that it is not data. That way we know what the variables are.

FLO-2DNoemi commented 8 months ago

Type4Culvert.txt includes the grid element and inlet name, this can introduce errors and data discrepancies, there are many times when the inlet is moved or some data is reentered, in this case, the user needs to update the Type4Culvert.txt, this is inconvenient. Not sure if the idea was to read the grid element from this ASCII file, but this is not a good idea.

Type I4 inlets are part of the SD shapefiles, we need to always pair the Type I4 with the grid element following the standard steps for inlets from the Select Components from Shapefile. In other words, when a user deletes the associated data (rating table or culvert) as Karen did, the inlet type 4 is still located in the same grid and this information comes from the inlet shapefile, so the grid element needs to be held in the table.

The only current way to relocate an inlet is by changing the location in the shapefiles and creating tables again from Select Components from Shapefile.

When the RT or Culvert data is deleted as Karen did in the video, the only information that needs to be entered at this point is either the culvert geometry or the RT. If the I4 inlet has culvert data, we should be able to fill the data doing one of the two following options:

1- Write the data in the table editor, this is convenient for modifying data for a single inlet or for small projects, this is inconvenient for large projects. 2- Use a file. I like the idea of using the same concept that we use for RT. In other words, one file per structure, only containing the necessary data, not grid elements, not inlet names. The file can have the inlet name to make sure it is paired with the correct inlet.

Data needed: CDIAMETER(I) TYPEC(I) TYPEEN(I) CUBASE(I)

Karen let me know if you have other ideas. Let's try to simplify this, so users need as little data processing as possible to build these files.

Noemi

-

The user will need to manually find the grid element and assign We need to be able to pair the grid element and inlet without needing this information from a ASCII file

On Sun, Mar 24, 2024 at 2:23 PM Juan Jose @.***> wrote:

@FLO-2DKaren https://github.com/FLO-2DKaren

I'll look at the issues 1) and 2) that you mentioned.

As for issue number 3) Import button (Add predefined... (folder icon)), the plugin looks for files with the same name of the inlet when a rating table is to be assigned to that inlet. But for Culvert equations the data should be in a predefined file named Type4Culvert.txt or Type4Culvert.dat (upper or lower cases is the same). Each line of Type4Culvert.txt defines the culvert equation for one inlet:

grid inlet cdiameter typec typeen cubase multbarrels

For instance,

imagen.png (view on web) https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/assets/20424575/f31b5b68-8575-4b81-af25-66645262ef67

— Reply to this email directly, view it on GitHub https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1092#issuecomment-2016893716, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE32O3V7JLZJGAA3EYQOGXLYZ4K3XAVCNFSM6AAAAABAE4LTVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJWHA4TGNZRGY . You are receiving this because you were mentioned.Message ID: @.***>

FLO-2DJJ commented 8 months ago

See my comment in red.

Juan Jose Rodriguez FLO-2D Software www.flo-2d.com @.***

On Mon, Mar 25, 2024 at 2:45 PM FLO-2DNoemi @.***> wrote:

Type4Culvert.txt includes the grid element and inlet name, this can introduce errors and data discrepancies, there are many times when the inlet is moved or some data is reentered, in this case, the user needs to update the Type4Culvert.txt, this is inconvenient. Not sure if the idea was to read the grid element from this ASCII file, but this is not a good idea.

The grid element number in Type4Culvert.txt is not used. The plugin first checks if the inlet name in Type4Culvert.txt exists in the Storm Drain Nodes table; if it does, the plugin uses the grid number from that table to assign it to the Culvert eq. table. If the inlet name from a Type4Culvert.txt line is not in the Storm Drain Nodes table, the line is skipped.

Type I4 inlets are part of the SD shapefiles, we need to always pair the Type I4 with the grid element following the standard steps for inlets from the Select Components from Shapefile. In other words, when a user deletes the associated data (rating table or culvert) as Karen did, the inlet type 4 is still located in the same grid and this information comes from the inlet shapefile, so the grid element needs to be held in the table.

The only current way to relocate an inlet is by changing the location in the shapefiles and creating tables again from Select Components from Shapefile.

When the RT or Culvert data is deleted as Karen did in the video, the only information that needs to be entered at this point is either the culvert geometry or the RT. If the I4 inlet has culvert data, we should be able to fill the data doing one of the two following options:

1- Write the data in the table editor, this is convenient for modifying data for a single inlet or for small projects, this is inconvenient for large projects.

This can be done making some adjustments to the current code:

2- Use a file. I like the idea of using the same concept that we use for

RT. In other words, one file per structure, only containing the necessary data, not grid elements, not inlet names. The file can have the inlet name to make sure it is paired with the correct inlet.

Data needed: CDIAMETER(I) TYPEC(I) TYPEEN(I) CUBASE(I)

If it is not a big problem I would prefer to use the Type4Culvert.txt file. Otherwise the code needs to be changed more than slightly. Instead we could eliminate the grid number from each line of Type4Culvert.txt and keep the rest: INLETNAME CDIAMETER TYPEC TYPEEN CUBASE

Karen let me know if you have other ideas. Let's try to simplify this, so users need as little data processing as possible to build these files.

Noemi

-

The user will need to manually find the grid element and assign We need to be able to pair the grid element and inlet without needing this information from a ASCII file

On Sun, Mar 24, 2024 at 2:23 PM Juan Jose @.***> wrote:

@FLO-2DKaren https://github.com/FLO-2DKaren

I'll look at the issues 1) and 2) that you mentioned.

As for issue number 3) Import button (Add predefined... (folder icon)), the plugin looks for files with the same name of the inlet when a rating table is to be assigned to that inlet. But for Culvert equations the data should be in a predefined file named Type4Culvert.txt or Type4Culvert.dat (upper or lower cases is the same). Each line of Type4Culvert.txt defines the culvert equation for one inlet:

grid inlet cdiameter typec typeen cubase multbarrels

For instance,

imagen.png (view on web) < https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/assets/20424575/f31b5b68-8575-4b81-af25-66645262ef67>

— Reply to this email directly, view it on GitHub < https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1092#issuecomment-2016893716>,

or unsubscribe < https://github.com/notifications/unsubscribe-auth/AE32O3V7JLZJGAA3EYQOGXLYZ4K3XAVCNFSM6AAAAABAE4LTVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJWHA4TGNZRGY>

. You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1092#issuecomment-2018042305, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE32O7ZK4VK42U7KOOPPB73Y2ATBPAVCNFSM6AAAAABAE4LTVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJYGA2DEMZQGU . You are receiving this because you were mentioned.Message ID: @.***>

FLO-2DNoemi commented 8 months ago

I do not see the comments in red in the email, so I am not sure if I caught all of them. See my input below:

1- The grid element number needs to be removed from Type4Culvert.txt. I was not aware this file was needed to read Culvert Type 4 inlets. The file needs to be added to the manuals. I do not see a problem using one single file for all the type 4 culverts. 2- Culvert data for type 4 needs to be editable in the table editor.

@Karen OBrien @.***> can add to my comments. She may have additional input based on her testing.

On Mon, Mar 25, 2024 at 3:01 PM Juan Jose @.***> wrote:

See my comment in red.

Juan Jose Rodriguez FLO-2D Software www.flo-2d.com @.***

On Mon, Mar 25, 2024 at 2:45 PM FLO-2DNoemi @.***> wrote:

Type4Culvert.txt includes the grid element and inlet name, this can introduce errors and data discrepancies, there are many times when the inlet is moved or some data is reentered, in this case, the user needs to update the Type4Culvert.txt, this is inconvenient. Not sure if the idea was to read the grid element from this ASCII file, but this is not a good idea.

The grid element number in Type4Culvert.txt is not used. The plugin first checks if the inlet name in Type4Culvert.txt exists in the Storm Drain Nodes table; if it does, the plugin uses the grid number from that table to assign it to the Culvert eq. table. If the inlet name from a Type4Culvert.txt line is not in the Storm Drain Nodes table, the line is skipped.

Type I4 inlets are part of the SD shapefiles, we need to always pair the Type I4 with the grid element following the standard steps for inlets from the Select Components from Shapefile. In other words, when a user deletes the associated data (rating table or culvert) as Karen did, the inlet type 4 is still located in the same grid and this information comes from the inlet shapefile, so the grid element needs to be held in the table.

The only current way to relocate an inlet is by changing the location in the shapefiles and creating tables again from Select Components from Shapefile.

When the RT or Culvert data is deleted as Karen did in the video, the only information that needs to be entered at this point is either the culvert geometry or the RT. If the I4 inlet has culvert data, we should be able to fill the data doing one of the two following options:

1- Write the data in the table editor, this is convenient for modifying data for a single inlet or for small projects, this is inconvenient for large projects.

This can be done making some adjustments to the current code:

  • When Adding a Culvert Equation [image: imagen.png] Currently it assigns a default 'CulvertEqn' name but we could ask for a user name.

  • Also when renaming the Culvert eq. [image: imagen.png]

2- Use a file. I like the idea of using the same concept that we use for

RT. In other words, one file per structure, only containing the necessary data, not grid elements, not inlet names. The file can have the inlet name to make sure it is paired with the correct inlet.

Data needed: CDIAMETER(I) TYPEC(I) TYPEEN(I) CUBASE(I)

If it is not a big problem I would prefer to use the Type4Culvert.txt file. Otherwise the code needs to be changed more than slightly. Instead we could eliminate the grid number from each line of Type4Culvert.txt and keep the rest: INLETNAME CDIAMETER TYPEC TYPEEN CUBASE

Karen let me know if you have other ideas. Let's try to simplify this, so users need as little data processing as possible to build these files.

Noemi

-

The user will need to manually find the grid element and assign We need to be able to pair the grid element and inlet without needing this information from a ASCII file

On Sun, Mar 24, 2024 at 2:23 PM Juan Jose @.***> wrote:

@FLO-2DKaren https://github.com/FLO-2DKaren

I'll look at the issues 1) and 2) that you mentioned.

As for issue number 3) Import button (Add predefined... (folder icon)), the plugin looks for files with the same name of the inlet when a rating table is to be assigned to that inlet. But for Culvert equations the data should be in a predefined file named Type4Culvert.txt or Type4Culvert.dat (upper or lower cases is the same). Each line of Type4Culvert.txt defines the culvert equation for one inlet:

grid inlet cdiameter typec typeen cubase multbarrels

For instance,

imagen.png (view on web) <

https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/assets/20424575/f31b5b68-8575-4b81-af25-66645262ef67>

— Reply to this email directly, view it on GitHub <

https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1092#issuecomment-2016893716>,

or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AE32O3V7JLZJGAA3EYQOGXLYZ4K3XAVCNFSM6AAAAABAE4LTVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJWHA4TGNZRGY>

. You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHub < https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1092#issuecomment-2018042305>,

or unsubscribe < https://github.com/notifications/unsubscribe-auth/AE32O7ZK4VK42U7KOOPPB73Y2ATBPAVCNFSM6AAAAABAE4LTVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJYGA2DEMZQGU>

. You are receiving this because you were mentioned.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1092#issuecomment-2018700983, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE32O3THHX2RBREFBVAUIXLY2BYCFAVCNFSM6AAAAABAE4LTVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJYG4YDAOJYGM . You are receiving this because you were mentioned.Message ID: @.***>

FLO-2DKaren commented 8 months ago

No worries Noemi, We'll sort those issues out.

2 is already finished.

  1. JJ's going to remove the grid from the table and allow us to add a 1 line header so we can identify each column.
FLO-2DNoemi commented 8 months ago

Great!

We also need to check the following:

When the type 4 Culvert data or RT data is deleted/modified in QGIS. The plugin should hold the grid element associated with the type 4. The grid element number associated with type 4 should not be modifiable with the Culvert/RT data. It comes from the inlet/junctions shapefile.

On Mon, Mar 25, 2024 at 8:03 PM Karen @.***> wrote:

No worries Noemi, We'll sort those issues out.

2 is already finished.

  1. JJ's going to remove the grid from the table and allow us to add a 1 line header so we can identify each column.

— Reply to this email directly, view it on GitHub https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1092#issuecomment-2019131005, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE32O3TB5RMM4KB47Z2KNPDY2C3LPAVCNFSM6AAAAABAE4LTVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJZGEZTCMBQGU . You are receiving this because you were mentioned.Message ID: @.***>

FLO-2DKaren commented 8 months ago

It shouldn't be a problem because it can be updated with the following: Schematize storm drain Set a culvert from the table editor Import a bunch of culverts

The grid data is already stored in the user layer and in the schema layer so that update is automatic.

FLO-2DNoemi commented 8 months ago

Yes, maybe it is not an issue but we need to test it. In the video that you recorded, the table was missing the grid element.

On Mon, Mar 25, 2024 at 8:27 PM Karen @.***> wrote:

It shouldn't be a problem because it can be updated with the following: Schematize storm drain Set a culvert from the table editor Import a bunch of culverts

The grid data is already stored in the user layer and in the schema layer so that update is automatic.

— Reply to this email directly, view it on GitHub https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1092#issuecomment-2019163183, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE32O3X45ODEHEGE3TOFNF3Y2C6IFAVCNFSM6AAAAABAE4LTVKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJZGE3DGMJYGM . You are receiving this because you were mentioned.Message ID: @.***>