NOAA-OWP / t-route

Tree based hydrologic and hydraulic routing
Other
44 stars 50 forks source link

Getting an error during singularity build #822

Closed benlee0423 closed 3 months ago

benlee0423 commented 3 months ago

Current behavior

Build t-route with latest code in our HPC singularity build. Getting the following error message, and build failed.

removing build/bdist.linux-x86_64/wheel
Traceback (most recent call last):
  File "/t-route/src/troute-routing/setup.py", line 9, in <module>
    import troute.network as tn
ModuleNotFoundError: No module named 'troute.network'

I think this is due to the change made on the issue #783 and this commit.

Expected behavior

Build without error

Steps to replicate behavior (include URLs)

  1. git clone -b ngen-latest https://github.com/CIROH-UA/NGIAB-HPCInfra.git
  2. cd singularity
  3. sudo singularity build ciroh-ngen-singularity.sif singularity_ngen.def

Screenshots

Screenshot 2024-08-07 at 9 08 58 PM
aaraney commented 3 months ago

Hi @benlee0423, in #804 we improved the packaging of t-route by introducing usage of pyproject.toml, the modern python package metadata standard. We also made a few changes to how compiler.sh works under the hood so that it is compliant. As a result, building wheel's using python setup.py bdist_wheel will no longer work. I think you should be able to update install_t_route.sh to the following to resolve the issues.

https://github.com/CIROH-UA/NGIAB-HPCInfra/blob/d8981cb9321aa0565b75182845226b49ad4a43f8/singularity/templates/t-route/install_t_route.sh#L26-L37

cd /t-route/src/troute-network
python -m build --no-isolation --wheel .

cd /t-route/src/troute-routing
python -m build --no-isolation --wheel .

# troute-config doesn't use setup.py, use build to make the wheel
cd /t-route/src/troute-config
python -m build . \

# troute-nwm doesn't use setup.py, use build to make the wheel
cd /t-route/src/troute-nwm
python -m build .

Im going to close this as resolved, but please re-open if you continue to have issues.

benlee0423 commented 3 months ago

@aaraney Your suggestion does not fix the issue. Need to reopened the issue. We have two separate builds, one for Docker and the other for Singularity. Docker - https://github.com/CIROH-UA https://github.com/CIROH-UA/NGIAB-CloudInfra/actions/runs/10344207385 HPC - https://github.com/CIROH-UA/NGIAB-HPCInfra https://github.com/CIROH-UA/NGIAB-HPCInfra/actions/runs/10343787782/job/28628389777

Docker build looks fine with latest t-route, but HPC repo failed with your suggestion. I think the change in #804 in compiler.sh file broke the build. The line 91 looks like compatible with singularity build, and it failed to compile troute-network, and consequently when it failed when it was imported during wheel build.

 if [[ ${WITH_EDITABLE} == true ]]; then
    pip install --no-build-isolation --config-setting='--build-option=--use-cython' --editable . --config-setting='editable_mode=compat' || exit
  else
    pip install --no-build-isolation --config-setting='--build-option=--use-cython' . || exit
  fi

Additionally, if I bring back t-route code to commit 59a6041, it builds without any error. In commit 59a6041, it uses the following code in compiler.sh https://github.com/NOAA-OWP/t-route/blob/59a604198bb272dbf86f0392c78625ac5402f5ac/compiler.sh#L90

if [[ "$build_framework" == true ]]; then
  #creates troute package
  cd $REPOROOT/src/troute-network
  rm -rf build
  ##python setup.py --use-cython install
  ##python setup.py --use-cython develop
  python setup.py build_ext --inplace --use-cython || exit
  pip install --no-build-isolation $E . || exit
fi
aaraney commented 3 months ago

Hey @benlee0423,

Ensure that you install t-route's python build time and runtime dependencies with pip install -r requirements.txt before running compiler.sh. It also looks like you might not have build installed per the above error: /usr/bin/python: No module named build.

Replace install_t_route.sh with the following and it should work.

#!/bin/bash

current_path=`pwd`

echo "==========================================================="
echo "-- Now loding modules ..."
echo "-----------------------------------------------------------"
source /etc/profile.d/modules.sh
module load mpi
module list

echo "==========================================================="
echo "==========================================================="

echo ""
echo "==========================================================="
echo "-- Now Cloning and Installing NOAA-OWP T-Route ..."
echo "-----------------------------------------------------------"
export FC=gfortran NETCDF=/usr/include
git clone --progress --single-branch --branch master https://github.com/NOAA-OWP/t-route
cd t-route
git submodule update --init

python -m pip install -r requirements.txt
python -m pip install build
./compiler.sh no-e

cd /t-route/src/troute-network
python -m build --no-isolation --wheel .

cd /t-route/src/troute-routing
python -m build --no-isolation --wheel .

# troute-config doesn't use setup.py, use build to make the wheel
cd /t-route/src/troute-config
python -m build --wheel .

cd /t-route/src/troute-nwm
python -m build --wheel .

cd /t-route
cp /t-route/src/troute-network/dist/*.whl /t-route/wheels
cp /t-route/src/troute-routing/dist/*.whl /t-route/wheels
cp /t-route/src/troute-config/dist/*.whl /t-route/wheels
cp /t-route/src/troute-nwm/dist/*.whl /t-route/wheels
benlee0423 commented 3 months ago

@aaraney I followed your suggestion, it still failed with the same error message.

Using old commit 59a6041, the build shows the following line in build log file.

Building wheels for collected packages: troute.network
  Building wheel for troute.network (PEP 517) ... done
  Created wheel for troute.network: filename=troute.network-0.0.0-cp39-cp39-linux_x86_64.whl size=1843079 sha256=152d44b650fd6a87baac712c5ba135d88da7d33892249e3cea43a107509a793e
  Stored in directory: /root/.cache/pip/wheels/34/1e/6d/941b245ccfbad5f7f5ff0d079c0e6006b31661a9461f9cf8ef
Successfully built troute.network
Installing collected packages: troute.network
Successfully installed troute.network-0.0.0

But, I don't see any of these in build log from latest t-route with your code change suggestion.

aaraney commented 3 months ago

Just opened a PR that should solve your issues. Please take a look and allow running the GH action.

It looks the GH action runner was not run on the branch that has the changes I suggested.

benlee0423 commented 3 months ago

@aaraney I did mistake on the branch. It worked with my modified branch with your suggestion. We can close this ticket. Thank you for your help.

aaraney commented 3 months ago

@benlee0423, great! Glad that got it sorted out!