firetools / qgis2fds

An open source and free tool to export terrain elevation, landuse, and georeferencing to NIST FDS for computational fluid dynamics (CFD) wildfire or atmospheric pollutants dispersion simulations.
GNU General Public License v3.0
17 stars 13 forks source link

qgis2fds session hangs #78

Closed mcgratta closed 1 year ago

mcgratta commented 1 year ago

I am trying to create an FDS input file from the case firemodels/cad/Case_Studies/Chimney_Tops_2/QGIS. When I run the qgis2fds script, the process hangs. I am using the dev branch.

johodges commented 1 year ago

Taking a look

johodges commented 1 year ago

I am able to reproduce the error on my machine. The case is hanging in the triangular interpolation routine. Still working on identifying the root cause of the issue within that routine.

mcgratta commented 1 year ago

This was working a few weeks ago with someone's branch.

johodges commented 1 year ago

In the prior develop branch (https://github.com/johodges/qgis2fds/tree/develop), I replaced the TIN interpolation routine entirely to bypass this issue. Unfortunately, the new dev branch was not based on that fork and I have had to migrate / re-implement some changes into the current dev branch. I have a fix implemented in https://github.com/firetools/qgis2fds/pull/79, which bypasses this routine the way the johodges/develop branch does. However, the output files change enough to fail the verification cases. I manually reviewed them and personally do not see any issue merging the PR, but I want Ema to review it before I merge. With his blessing I'll update the figures in the figure repo so the PR will pass.

emanuelegissi commented 1 year ago

The dev branch is by definition unstable and not ready for production. Please, stick to the version that was working, the official stable one, that has been working well for many users, or any following fork as you please.

As Jonathan writes, unfortunately, the new dev branch could not be based on his fork, because IMHO that code had good ideas in but was a broken mess. (Am I too explicit now?)

I tried hard to nudge Jonathan to atomic PRs to keep everybody enthusiasm on board, but with limited success: nobody likes reviewing PRs of 1000 lines touching right and left.

But no good deed goes unpunished, as Kevin was writing some days ago.

Unfortunately again, the current state of the dev branch is far from stable. Today not even the verification was working on my laptop, I dedicated several hours to that.

I am available for a call tomorrow at 2pm European time to try to quickly fix Kevin issue. Let me know.

It is 1145 pm here, time for sleeping.

emanuelegissi commented 1 year ago

Even 3 or 4 pm is okay

rmcdermo commented 1 year ago

Ema,

To my knowledge, the main branch does not work for us. That is what launched us in this path all summer. So, we here at NIST seem to be in trouble at the moment.

Am I wrong on this?

On Wed, Aug 16, 2023 at 5:47 PM Emanuele Gissi @.***> wrote:

The dev branch is by definition unstable and not ready for production. Please, stick to the version that was working, the official stable one, that has been working well for many users, or any following fork as you please.

As Jonathan writes, unfortunately, the new dev branch could not be based on his fork, because IMHO that code had good ideas in but was a broken mess. (Am I too explicit now?)

I tried hard to nudge Jonathan to atomic PRs to keep everybody enthusiasm on board, but with limited success: nobody likes reviewing PRs of 1000 lines touching right and left.

But no good deed goes unpunished, as Kevin was writing some days ago.

Unfortunately again, the current state of the dev branch is far from stable. Today not even the verification was working on my laptop, I dedicated several hours to that.

I am available for a call tomorrow at 2pm European time to try to quickly fix Kevin issue. Let me know.

It is 1145 pm here, time for sleeping.

— Reply to this email directly, view it on GitHub https://github.com/firetools/qgis2fds/issues/78#issuecomment-1681312616, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBWXQP2CDJYYXX4B3AEXZTXVU5X3ANCNFSM6AAAAAA3SYUKEU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Sent from my iPhone

emanuelegissi commented 1 year ago

You are in trouble, and I am available.

johodges commented 1 year ago

Thanks Ema. I can walk you through the changes tomorrow and the differences in output I noticed when reviewing the results from the verification. I'll send out a meeting invite for 2pm EU time.

emanuelegissi commented 1 year ago

Jonathan, we cannot simply replace interpolation with reprojecting and warping. A picture is better than 1000 words: immagine

In Kevin case it seems that the interpolation is stuck somewhere. I have been trying since early morning to understand why. The alternative is writing an interpolating algorithm from scratch, not impossible and not quick.

I keep exploring easier alternatives.

emanuelegissi commented 1 year ago

TIN interpolation is still the best method (when it works ;-)

Current alternative candidates:

  1. GDAL/Raster analysis Grid/IDW (https://docs.qgis.org/3.28/en/docs/user_manual/processing_algs/gdal/rasteranalysis.html#grid-idw-with-nearest-neighbor-searching), disadvantage: setting pixel size seems not easy, but feasible
  2. create contours (GDAL/Raster extraction/Contour) → create new raster (GRASS r.surf.contour), disadvantage: it needs GRASS
emanuelegissi commented 1 year ago

Alternative 1. If GDAL version > 3.2, setting the pixel size is easy: -tr xres yres

emanuelegissi commented 1 year ago

It seems that GDAL/Raster analysis Grid/IDWnn is the right alternative:

qgis_process run gdal:gridinversedistancenearestneighbor --distance_units=meters --area_units=m2 --ellipsoid=EPSG:7019 --INPUT='/home/egissi/Documenti/Git/firemodels/cad/Case_Studies/Chimney_Tops_2/QGIS/Tmp/Raster to points UTM.gpkg' --Z_FIELD=DEM --POWER=2 --SMOOTHING=0 --RADIUS=30 --MAX_POINTS=4 --MIN_POINTS=1 --NODATA=-999 --OPTIONS= --EXTRA='-txe 4152751.8 4751578.1 -tye 4157399.7 4755090.6 -tr 10.0 10.0' --DATA_TYPE=5 --OUTPUT='/home/egissi/Documenti/Git/firemodels/cad/Case_Studies/Chimney_Tops_2/QGIS/Tmp/Interpolated GDAL GRID IDW.tif'

emanuelegissi commented 1 year ago

Now lunch, then implement

emanuelegissi commented 1 year ago

Jonathan, while I am doing that you could add the a curated selection of firemodels/cad/Case_Studies/ to our verification suite to prevent future bad surprises

emanuelegissi commented 1 year ago

Chimney tops seems to work now. Let me do some verification, commit and push to dev branch so that you can test from your side

emanuelegissi commented 1 year ago

Verification machinery does not work yet locally. Committed and pushed. Fingers crossed.

emanuelegissi commented 1 year ago

Chimney tops, in Smokeview: immagine

johodges commented 1 year ago

thanks for the update. I just added the chimney tops case to the actions workflow. I'll check with your latest commit once that finishes.

mcgratta commented 1 year ago

Feel free to modify anything in the Chimney_Tops folder to fix the problem or make the case set up more compatible with your verification cases. My only requirement is that there be two sub-folders named QGIS and FDS. If the end user just wants to run the FDS case, he/she can just go to the FDS folder. The machinery within the QGIS folder can create the .fds, .bingeom, and texture file that go into the FDS folder.

emanuelegissi commented 1 year ago

Okay, thank you.

(Now, the fire layer feature is back in dev, too. So Chimney_Tops should fully work.)

johodges commented 1 year ago

The chimney tops case now works without hanging and passes verification after merging https://github.com/firetools/qgis2fds/pull/81

mcgratta commented 1 year ago

Which branch do I use?

johodges commented 1 year ago

dev on the main repo: https://github.com/firetools/qgis2fds/tree/dev

I have the updated project file and layers here: https://github.com/firetools/qgis2fds/tree/dev/verification/tests/test_chimney_tops_2

Do you want me to mirror over to the cad repo as well?

rmcdermo commented 1 year ago

@johodges I would suggest that CAD be the source. You can link to that from within your workflow on Actions.

So, yes, whenever you have things working. Please push the necessary QGIS files, etc., to CAD.