Skip to content

Correctly apply v3.6.0 (Ubuntu 24.04) patch#1956

Closed
spwoodcock wants to merge 2 commits intoOpenDroneMap:masterfrom
spwoodcock:fix/3.6.0
Closed

Correctly apply v3.6.0 (Ubuntu 24.04) patch#1956
spwoodcock wants to merge 2 commits intoOpenDroneMap:masterfrom
spwoodcock:fix/3.6.0

Conversation

@spwoodcock
Copy link
Copy Markdown
Member

  • No idea what happened in Merge branch 24.04 --> master (#1904) #1942
  • I added all the changes on the 24.04 branch --> master, but it seems to have not fully applied everything (perhaps my fork was out of sync, but I didn't think so at the time - anything is possible!)
  • This PR re-applies all the changes made for the 3.6.0 release, which I believe is a working tag currently.
  • This should bring master in line with 3.6.0, so hopefully everything works 🤞

Related to #1949

Note

If this doesn't work, then in the worst case we could manually reset master HEAD to before these changes, then cherry-pick exactly what we needed on top, and force push.

smathermather and others added 2 commits October 22, 2025 16:25
* Changes to support building on Ubuntu 24.04 and windows-2022 Github runner.
* Update Python to 3.12
    * Install dependencies in virtual environment
    * Run python scripts from the virtual environment)
* Update dependencies
    * Ubuntu dependencies in snap/snapcraft24.yaml
    * Python dependencies in requirements.txt
    * Windows dependencies built in OpenDroneMap/windows-deps repo
* Update CUDA
    * 12.8.1 on Windows
    * 13.0.0 on Ubuntu)
* Run tests as part of docker build
* Use exact commits to specify dependencies that are built from source, instead of branch names
* Use upstream versions of Libraries:
    * PDAL
    * PDAL-Python
    * untwine
    * ExifRead
    * draco
* Build windows builds with -j2
* Use Micasense's latest version of dls.py
* Update failing unit tests to match current behavior
@spwoodcock spwoodcock changed the title Correctly apply v3.6.0 patch Correctly apply v3.6.0 (Ubuntu 24.04) patch Nov 19, 2025
@spwoodcock
Copy link
Copy Markdown
Member Author

git diff origin/fix/3.6.0 upstream/24.04
diff --git a/requirements.txt b/requirements.txt
index 11eb8db..54775b1 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -10,7 +10,6 @@ joblib==1.5.1
 matplotlib==3.10.3
 numpy==2.3.2
 Pillow==11.3.0
-pillow-jxl-plugin==1.3.4
 vmem==1.0.2
 pyodm==1.5.12
 pyproj==3.7.1
diff --git a/snap/snapcraft24.yaml b/snap/snapcraft24.yaml
index 6028bf1..6e3f4d6 100644
--- a/snap/snapcraft24.yaml
+++ b/snap/snapcraft24.yaml
@@ -79,7 +79,6 @@ parts:
       - libflann-dev
       - libgtk2.0-dev
       - libjpeg-dev
-      - libjxl-dev
       - liblapack-dev
       - libopenjpip7
       - libpng-dev
@@ -95,7 +94,6 @@ parts:
       - libflann1.9
       - libgtk2.0-0
       - libjpeg-turbo8
-      - libjxl0.7
       - libopenjpip7
       - liblapack3
       - libpng16-16

The master should basically match 24.04 after this. The two small differences in the diff above are from c433802, as there was a merge conflict without adding these additional libs in.

Merging shouldn't change these lines on master, as it already has the libs present:

pillow-jxl-plugin==1.3.4

@smathermather smathermather self-assigned this Nov 19, 2025
@smathermather smathermather self-requested a review November 19, 2025 15:16
@smathermather
Copy link
Copy Markdown
Contributor

Excellent. Will test this afternoon and revert.

Copy link
Copy Markdown
Contributor

@smathermather smathermather left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs this change to address lack of overwrite on point cloud classification (add overwrite flag to line 163 of opendm/dem/pdal.py):
https://github.com/OpenDroneMap/ODM/pull/1950/files

But even then the python gdal issue remains:

[INFO]    running "/code/venv/bin/python3" "/code/opendm/tiles/hsv_merge.py" "/code/odm_dem/dtm.previewcolor.tif" "/code/odm_dem/dtm.previewhillshade.tif" "/code/odm_dem/dtm.previewcolored_hillshade.tif"
/code/venv/lib/python3.12/site-packages/osgeo/gdal.py:312: FutureWarning: Neither gdal.UseExceptions() nor gdal.DontUseExceptions() has been explicitly called. In GDAL 4.0, exceptions will be enabled by default.
  warnings.warn(
Traceback (most recent call last):
  File "/code/opendm/tiles/hsv_merge.py", line 198, in <module>
    rScanline = rBand.ReadAsArray(0, i, hillband.XSize, 1, hillband.XSize, 1)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/code/venv/lib/python3.12/site-packages/osgeo/gdal.py", line 5283, in ReadAsArray
    from osgeo import gdal_array
  File "/code/venv/lib/python3.12/site-packages/osgeo/gdal_array.py", line 10, in <module>
    from . import _gdal_array
ImportError: cannot import name '_gdal_array' from 'osgeo' (/code/venv/lib/python3.12/site-packages/osgeo/__init__.py)
[WARNING] Cannot generate colored hillshade: Child returned 1
[INFO]    running gdal_translate -outsize 1400 0 -of png "None" "/code/opensfm/stats/dtm.png" --config GDAL_CACHEMAX 46.6%
ERROR 4: None: No such file or directory

===== Dumping Info for Geeks (developers need this to fix bugs) =====
Child returned 1
Traceback (most recent call last):
  File "/code/stages/odm_app.py", line 82, in execute
    self.first_stage.run()
  File "/code/opendm/types.py", line 470, in run
    self.next_stage.run(outputs)
  File "/code/opendm/types.py", line 470, in run
    self.next_stage.run(outputs)
  File "/code/opendm/types.py", line 470, in run
    self.next_stage.run(outputs)
  [Previous line repeated 8 more times]
  File "/code/opendm/types.py", line 449, in run
    self.process(self.args, outputs)
  File "/code/stages/odm_report.py", line 214, in process
    system.run("gdal_translate -outsize {} 0 -of png \"{}\" \"{}\" --config GDAL_CACHEMAX {}%".format(image_target_size, colored_hillshade_dem, osfm_dem, get_max_memory()))
  File "/code/opendm/system.py", line 112, in run
    raise SubprocessException("Child returned {}".format(retcode), retcode)
opendm.system.SubprocessException: Child returned 1

@smathermather
Copy link
Copy Markdown
Contributor

Note

If this doesn't work, then in the worst case we could manually reset master HEAD to before these changes, then cherry-pick exactly what we needed on top, and force push.

I'm not comfortable letting the master branch sit broken for much longer, as we are formally rolling release.

Perhaps we do follow the path of worst case and resolve commit by commit.

@smathermather
Copy link
Copy Markdown
Contributor

Unsurprisingly, by contrast this finishes successfully

git reset --hard 0080422
docker build -t 008 .
cd ~/Documents/git/drone_dataset_brighton_beach
docker run -it --rm -v "$(pwd)/images:/code/images" -v "$(pwd)/odm_orthophoto:/code/odm_orthophoto" -v "$(pwd)/odm_texturing:/code/odm_texturing" 008 --dtm --rerun-all

...
[INFO]    Finished odm_report stage
[INFO]    Running odm_postprocess stage
[INFO]    Post Processing
[INFO]    Finished odm_postprocess stage
[INFO]    No more stages to run
...
[INFO]    ODM app finished - Wed Nov 19 21:39:09  2025

Not sure the best procedure for cherry picking, but happy to do so and test each commit in sequence with guidance.

@smathermather
Copy link
Copy Markdown
Contributor

Not sure the best procedure for cherry picking, but happy to do so and test each commit in sequence with guidance.

Looks like it's mostly just one relevant pull request from Nathan adding JPEG XL support and his branch is still out there, the consequential 24.04 one aside:

756cb08 (HEAD -> master, origin/master, origin/HEAD) smrf overwrite (#1950)
c433802 Add JPEG XL support (#1945)
ff00047 24.04 (#1904) (#1942)
0080422 feat: add flag --merge-skip-blending to skip expensive blending in splitmerge (#1934)

But, I don't know norms around hard pushes on remote branches well enough. My temptation would be:

git reset --hard 0080422
git push --force origin master

And then open back up and pull in the JPEG XL pull request, and then we work from there on sorting 24.04.

But definitely interested in thoughts from @spwoodcock and @NathanMOlson.

@NathanMOlson
Copy link
Copy Markdown
Contributor

Don't try to merge the JPEG XL PR before 24.04, it depends on the 24.04 work.

@smathermather
Copy link
Copy Markdown
Contributor

Don't try to merge the JPEG XL PR before 24.04, it depends on the 24.04 work.

Ah, of course.

@spwoodcock
Copy link
Copy Markdown
Member Author

Yeah something is bodged in the commits & I think the easiest approach is a reset & merging / cherry-picking what we need - I'll take a look today 👍

(and will run the same tests on the final code)

@spwoodcock spwoodcock closed this Nov 20, 2025
@spwoodcock
Copy link
Copy Markdown
Member Author

spwoodcock commented Nov 20, 2025

My assumption was that something went wrong with the merge, but I don't think that's the case (I have done a fair bit of diffing and manual inspection and things seem to be in place).

Could it actually be an error with the 24.04 upgrade?
It looked to me that all the checks passed during testing in #1904 though right?

Steps from here

1. Quick fix

  • I'll build master and take a poke around - if anything obvious stands out, I'll try and fix it to succeed with the brighton beach dataset above.

2. Revert

If no luck in the next few hours, I'll make a quick PR to revert both:

3. Investigate

Then I'll do some investigation / testing of 24.04, master, 0080422 (before the merge) and see if I can find the issue.

We can bundle the JPEG XL PR in with the 24.04 upgrade if that's ok @NathanMOlson 🙏

@spwoodcock
Copy link
Copy Markdown
Member Author

I have a feeling the issue is this:

ODM/requirements.txt

Lines 23 to 25 in 756cb08

gdal[numpy]==3.8.4 ; sys_platform == 'linux'
gdal[numpy]==3.8.4 ; sys_platform == 'darwin'
https://github.com/OpenDroneMap/windows-deps/releases/download/2.6.0/gdal-3.11.1-cp312-cp312-win_amd64.whl ; sys_platform == 'win32'

For windows we are installing GDAL 3.11.1, but for MacOS and Linux we install 3.8.4.

3.8.4 is actually the correct version for Ubuntu 24.04, however in order to install this version correctly we need an older version of numpy<2.
Installing an older version of numpy breaks the PoissonRecon build!

As the underlying system gdal version does appear to be 3.11.1, hence the errors above when accessing the shared modules.
Perhaps it was a typo! But I'm hoping the solution is to simply upgrade the Python gdal bindings version to 3.11.1 😄

Building and testing now 👍

@spwoodcock
Copy link
Copy Markdown
Member Author

spwoodcock commented Nov 20, 2025

ok that failed to build - makes sense as we install libgdal from Ubuntu 24.04, which will clearly be 3.8.4 🤦

Will take another route.

I also see you have already been here! #1904 (comment)
Sorry for the dup content

@spwoodcock
Copy link
Copy Markdown
Member Author

spwoodcock commented Nov 20, 2025

I think I found the issue!

We are extracting dependencies from the snapcraft yamls.
In snapcraft.yaml we have:

package-repositories:
  - type: apt
    ppa: ubuntugis/ubuntugis-unstable
    # Ensure prioritised above standard repo for GDAL version 3.11.1
    priority: 1001

I added the priority as part of my debugging. But it's not read either way - could possibly be removed

However, when the use configure.sh to do the dependency install, we are simply parsing the snapcraft yaml to get dependencies, meaning we don't add the ubuntugis/ubuntugis-unstable PPA to correctly get the latest version of GDAL.

I'm hoping updating configure.sh to add the PPA will fix this - testing a build now.

@spwoodcock
Copy link
Copy Markdown
Member Author

Hopefully:

    UBUNTU_VERSION=$(lsb_release -r)
    if [[ "$UBUNTU_VERSION" == *"20.04"* ]]; then
        echo "Enabling PPA for Ubuntu GIS"
        sudo $APT_GET install -y -qq --no-install-recommends software-properties-common
        sudo add-apt-repository ppa:ubuntugis/ppa
        sudo $APT_GET update
    elif [[ "$UBUNTU_VERSION" == *"24.04"* ]]; then
        echo "Enabling ubuntugis-unstable PPA for Ubuntu 24.04"
        sudo $APT_GET install -y -qq --no-install-recommends software-properties-common
        sudo add-apt-repository -y ppa:ubuntugis/ubuntugis-unstable
        sudo $APT_GET update
    fi

@spwoodcock
Copy link
Copy Markdown
Member Author

I wonder if anyone would be opposed to basing this on Debian Trixie instead of Ubuntu 24.04 going forward?

https://packages.debian.org/source/trixie/gdal
GDAL is officially on 3.10.3 without resorting to unable ppas.

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.

3 participants