Update sourceList.txt files to match capitalization of NVEL files.
Working on Linux or MacOSX to build FVS from source, we can pull the source code including the NVEL submodule now using
git clone --recurse-submodules https://github.com/USDAForestService/ForestVegetationSimulator.git and retrieve the NVEL code that was recently moved into the ForestVegetationSimulator/volume/NVEL folder as a submodule. However, many of the files within the NVEL repo are named with a mix of capitalizations while the FVS*_sourceList.txt files specify all these filenames to completely lower-cased.
Recommend updating the FVS*_sourceList.txt files to use the capitalization matching incoming NVEL repo rather then requiring the user to change all the filenames to lowercase to use existing sourceLists or to update sourceLists themselves.
Here is what currently happens (building using Windows Subsystem for Linux):
>> git clone --recurse-submodules https://github.com/USDAForestService/ForestVegetationSimulator.git
>> cd ForestVegetationSimulator/bin
>> make FVSpn
OS: ; OSTYPE: ; WIN: -Dunix ; OSARCH: l64; SLIBSUFX: .so
PIC: -fPIC; PRGSUFX:
mingw64: ; FCprf: ; CCprf:
FVSprgs: FVSak FVSbc FVSbm FVSca FVSci FVScr FVScs FVSec FVSem FVSie FVSkt FVSls FVSnc FVSne FVSoc FVSon FVSop FVSpn FVSsn FVSso FVStt FVSut FVSwc FVSws
CANprgs: FVSbc FVSon
USprgs: FVSak FVSbm FVSca FVSci FVScr FVScs FVSec FVSem FVSie FVSkt FVSls FVSnc FVSne FVSoc FVSop FVSpn FVSsn FVSso FVStt FVSut FVSwc FVSws
make: *** No rule to make target '../volume/NVEL/beqinfo.inc', needed by 'FVSpn'. Stop.
When all the files in the NVEL folder are renamed to be lower case, FVS builds successfully.
Did you rest that? I don't think it will work unless the file names in the include statements matche the names on the file system, exactly includine the case of each letter in the name This is the situation on systems with case sensitive file systems.
What really needs to happen, and is long overdue, is that the volume library needs to be changed so the the names exactly match.
On Mon, May 20, 2024, 10:25 PM wagnerds @.***> wrote:
We are aware of this issue and from our discussions with NVEL, it seems the "INCLUDE" statements within the NVEL files must also match the case of the actual filenames when in a Linux or MacOS environment. Currently, all the "INCLUDE" statements within NVEL are all listed as lowercase, so for the time being, our recommendation to those building in a Unix environment to use the following script in R to automatically convert the file naming case convention.
setwd("C:/code_repos/ForestVegetationSimulator/") fn = dir('./volume/NVEL/') fn = paste0('./volume/NVEL/',fn)
file.rename(fn, tolower(fn))
— Reply to this email directly, view it on GitHub https://github.com/USDAForestService/ForestVegetationSimulator/issues/44#issuecomment-2121150768, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVTTTN5VNEOFSNONL432D3ZDJL2ZAVCNFSM6AAAAABIAIK27WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRRGE2TANZWHA . You are receiving this because you are subscribed to this thread.Message ID: <USDAForestService/ForestVegetationSimulator/issues/44/2121150768@ github.com>
After trying the renaming of a NVEL files within a single sourceList.txt file, I can confirm @nickcrookston was right, and the build fails with the same error, first triggered by BEQINFO.inc
Agree that fixing this upstream would be ideal. That being said, I'd trust y'all's better judgement whether any of you could submit a pull request to NVEL to get that moving or if NVEL would accept a pull request from someone like me. Perhaps in the meantime y'all could make a fork of the NVEL library, update the file names there, and then have that version of the NVEL equations employed in this repo instead of using the main NVEL repo as a submodule? For example, maybe you could update the FMSC-Measurements/VolumeLibrary repo?
I'll have another discussion with NVEL. Last time I brought it up, they do not accept pull requests and prefer to do things via email.
Is there a suggested "hack" fix in the meantime? We have an automatic build system and this broke our workflow.
Update: what's even weirder is I've been rolling back the SHA I'm using to try to build off an earlier version of this (we were able to do the build a few months back) and I can't get it to build no matter what SHA I use (e.g. I'm back to using January 2024 commit but that's giving me the same error). Is this connected to a different github repo or something for those files? I don't understand why rolling back my pull isn't working to get this functional again.
@jgrn307 What was the source of the SHA you are reverting to? Was is just based off the date listed in this repository or from a source you previously had working? I suspect the SHA you may be referencing from January was the state of our code base in our Enterprise repository from that date, where the submodule repository change was already in effect. We held off on releasing that update until now as we only have Windows machines to test on.
Are you able to redirect your build to a previous tagged release? Maybe https://github.com/USDAForestService/ForestVegetationSimulator/tree/FS2023.3 ?
Also, what is your build environment? Some version of Linux I assume?
@jgrn307 @d-diaz
I may have misunderstood a previous comment. @d-diaz You said it built successfully when you changed the NVEL files to lowercase. I had posted an R script that should help users convert their local copy of the NVEL files to all lowercase, and may have misunderstood the response.
You said you renamed the files in the FVSxx_SourceList.txt to match the incoming NVEL files, and that didn't work as the file names did not match the NVEL 'INCLUDE' statements, which makes sense, but did you try the script I included that would convert the NVEL files to lowercase to match the 'INCLUDE' statements as well as the sourceList.txt file names?
Can you help me confirm whether running this script in R changes the file names within the volume/NVEL/ directory to lowercase within you local copy of NVEL?
setwd("PathToRepository/ForestVegetationSimulator/")
fn = dir('./volume/NVEL/')
fn = paste0('./volume/NVEL/',fn)
file.rename(fn, tolower(fn))
This seems to work in my instance of WSL, so I am looking for a bit of feedback on if this is a viable workaround.
I was able to get the build working by reverting to https://github.com/USDAForestService/ForestVegetationSimulator/commit/3b09ae2643866ec88149dabb91ae6b1bd5626f86 without the aforementioned error. I'm doing a linux-based Docker build of FVS.
I'll try the other tag tomorrow when I get back to this!
--j
@jgrn307 What was the source of the SHA you are reverting to? Was is just based off the date listed in this repository or from a source you previously had working? I suspect the SHA you may be referencing from January was the state of our code base in our Enterprise repository from that date, where the submodule repository change was already in effect. We held off on releasing that update until now as we only have Windows machines to test on.
Are you able to redirect your build to a previous tagged release? Maybe https://github.com/USDAForestService/ForestVegetationSimulator/tree/FS2023.3 ?
Also, what is your build environment? Some version of Linux I assume?
@jgrn307 @d-diaz
I may have misunderstood a previous comment. @d-diaz You said it built successfully when you changed the NVEL files to lowercase. I had posted an R script that should help users convert their local copy of the NVEL files to all lowercase, and may have misunderstood the response.
You said you renamed the files in the FVSxx_SourceList.txt to match the incoming NVEL files, and that didn't work as the file names did not match the NVEL 'INCLUDE' statements, which makes sense, but did you try the script I included that would convert the NVEL files to lowercase to match the 'INCLUDE' statements as well as the sourceList.txt file names?
Can you help me confirm whether running this script in R changes the file names within the volume/NVEL/ directory to lowercase within you local copy of NVEL?
setwd("PathToRepository/ForestVegetationSimulator/")fn = dir('./volume/NVEL/')fn = paste0('./volume/NVEL/',fn)file.rename(fn, tolower(fn))This seems to work in my instance of WSL, so I am looking for a bit of feedback on if this is a viable workaround.
I implemented a lower casing step in a bash shell script and was able to resume builds for US modules. Because the source lists for Canadian variants haven’t been updated to point to new NVEL paths, they are still failing:
cd ~/projects
git clone --recurse-submodules https://github.com/USDAForestService/ForestVegetationSimulator.git
cd ~/projects/ForestVegetationSimulator/volume/NVEL
rename 'y/A-Z/a-z/'
cd ~/projects/ForestVegetationSimulator/bin
make US -j12
mv FVS??.so /usr/local/lib
mv FVS?? /usr/local/bin
cd ~/projects
rm -rf ForestVegetationSimulator
This is a duct-tape fix. Ideally FVS source code will be provided in a way that doesn’t require these kinds of patches by the user to build.
Even if y’all are working on windows, would encourage using WSL or Docker to run build tests. As I mentioned above, to fix this without waiting for NVEL, you could incorporate a different repo as the submodule, such as a fork of the NVEL repo where y’all have already lower-cased the file names.
Any update on potential upstream fixes to build error due to capitalizations within volume/NVEL? If NVEL isn't likely to address these capitalization issues at source, an FVS fork of the NVEL repo with capitalization fixed so that you could incorporate the forked version as a submodule would be a relatively lightweight fix.
I wanted to re-ping this -- I'm still getting this error without the aforementioned fix or reverting back to an earlier version. Any chance this could get fixed asap?
We are discussing this item again as a group and seeing what kind of solution we can arrive at that will work more seamlessly for our users. More information coming soon. Thanks!
For what its worth, here is the Dockerfile we use to build FVS. Note that I'm stuck with "git checkout 3b09ae2" until the error gets fixed. I want to agree with d-diaz re: using Docker (and even maintaining one) to test this out "cleanly". It's easy enough to work with. Happy to help if you all need some!
# Use the official Python image.
FROM python:3.9-slim
# Set the application directory and work directory
ENV APP_HOME=/app
WORKDIR $APP_HOME
# Install system dependencies and clear cache
RUN apt-get update && apt-get install -y \
gfortran \
cmake \
unixodbc-dev \
build-essential \
git \
&& rm -rf /var/lib/apt/lists/*
# Clone and compile FVS (3b09ae2 commit for stability)
RUN git clone https://github.com/USDAForestService/ForestVegetationSimulator.git && \
cd ForestVegetationSimulator && \
git checkout 3b09ae2
# Compile the FVS binary, keeping this as a separate layer for caching
RUN cd ForestVegetationSimulator/bin && make
# Update PATH environment to include the FVS binary
ENV PATH="/app/ForestVegetationSimulator/bin:$PATH"
As mentioned before, if you don't do the older checkout, when building the Docker using the latest you get:
=> ERROR [5/5] RUN cd ForestVegetationSimulator/bin && make 0.5s
[5/5] RUN cd ForestVegetationSimulator/bin && make:
0.435 OS: ; OSTYPE: ; WIN: -Dunix ; OSARCH: l64; SLIBSUFX: .so
0.435 PIC: -fPIC; PRGSUFX: 0.435 mingw64: ; FCprf: ; CCprf: 0.440 FVSprgs: FVSak FVSbc FVSbm FVSca FVSci FVScr FVScs FVSec FVSem FVSie FVSkt FVSls FVSnc FVSne FVSoc FVSon FVSop FVSpn FVSsn FVSso FVStt FVSut FVSwc FVSws 0.440 CANprgs: FVSbc FVSon 0.440 USprgs: FVSak FVSbm FVSca FVSci FVScr FVScs FVSec FVSem FVSie FVSkt FVSls FVSnc FVSne FVSoc FVSop FVSpn FVSsn FVSso FVStt FVSut FVSwc FVSws 0.469 make: *** No rule to make target '../volume/NVEL/beqinfo.inc', needed by 'FVSak.so'. Stop.
Dockerfile:22
20 |
21 | # Compile the FVS binary, keeping this as a separate layer for caching
22 | >>> RUN cd ForestVegetationSimulator/bin && make
23 |
24 | # Update PATH environment to include the FVS binary
ERROR: failed to solve: process "/bin/sh -c cd ForestVegetationSimulator/bin && make" did not complete successfully: exit code: 2
- The terminal process "/usr/bin/bash '-c', 'docker build --pull --rm -f "200_fvs_execution/210_run_fvs/standalone_fvs_docker/Dockerfile" -t fvs "200_fvs_execution/210_run_fvs/standalone_fvs_docker"'" terminated with exit code: 1.
- Terminal will be reused by tasks, press any key to close it.
This is all super helpful. Thank you @d-diaz for opening and @wagnerds for your support.
One more idea - it would be awesome if you guys could use GitHub actions to verify that new releases can be built on whatever OS's you want to verify. Happy to help with the setup of those actions. Actions are a nice way to automate the testing (and anything else you want to do) prior to releasing new versions.
I also would love to see a docker container or even just a DOCKERFILE in this repo as well as suggested by @jgrn307.
Update: getting Floating point exception while running the tests in ./tests
Update2: False alarm, that's just a different issue #49 . After applying the fix listed there, no floating point exceptions, but some tests do not pass, probably outdated tests. FVSso for example all tests passed.
I used this command to list out the files that are used in the source code ( use and include directives), then cross listed with their paths (executed from within the repo root directory)
grep -hiR -e "^[[:space:]]*use " -e "^[[:space:]]*include " **/*.[fF]* **/*.[iI][nN][cC] \
| sed -E 's/^[[:space:]]*([uU][sS][eE]|[iI][nN][cC][lL][uU][dD][eE])[[:space:]]+//' \
| sed -E "s/^['\"]+//" \
| sed -E "s/['\"].*//" \
| sed -E "s/,.*//" \
| sed -E "s/[[:space:]]*$//" \
| sort | uniq | while IFS= read -r file; do
if ! find . -type f -name "$file" -print -quit | grep -q .; then
echo "$file"
fi
done
Changing -name to -iname shows the files that have mismatched spellings,
Only two files show up, so I copied them with the right case:
cp ./volume/NVEL/files_not_in_vollib/fvs_fiajsp.inc ./volume/NVEL/files_not_in_vollib/FVS_FIAJSP.INC
cp ./volume/NVEL/files_not_in_vollib/fiasp_sgmcbp.inc ./volume/NVEL/files_not_in_vollib/FIASP_SGMCBP.INC
(DBSTYPEDEFS.F77 was also listed, but it is a target, not an actual source file, and its generation and use cases are consistent).
Next was updating the cmake sourceList, with the help of chatgpt of going through the lines and checking if they exist on disk with the same case, if not, adjust the line to reflect the case
I added this change_cases.sh file to the ./bin directory and executing sh change_cases.sh
#!/bin/sh
for list in *_sourceList.txt; do
echo "Processing $list..."
tmp=$(mktemp)
while IFS= read -r filepath; do
# Skip empty lines
if [ -z "$filepath" ]; then
echo "" >> "$tmp"
continue
fi
# Extract directory and filename
dir=$(dirname "$filepath")
base=$(basename "$filepath")
# Try to locate the file with the correct case.
if [ -d "$dir" ]; then
# Look only in the specified directory (non-recursively)
correct=$(find "$dir" -maxdepth 1 -type f -iname "$base" -print -quit)
else
# Fall back to searching the whole tree if the directory doesn't exist
correct=$(find . -type f -iname "$base" -print -quit)
fi
# If a match is found, output the corrected path; otherwise, keep original.
if [ -n "$correct" ]; then
echo "$correct" >> "$tmp"
else
echo "$filepath" >> "$tmp"
echo "$filepath" 1>&2
fi
done < "$list"
mv "$tmp" "$list"
done
No stderr output, means all files accounted for.
I know this is quick and dirty, especially the copying part, (ideally each variant would cross-list its sourceList.txt file for the correct INCLUDE/USE), but so far no compilation or linking errors.
PS: I am running this natively on my arch linux, not within docker.
0- git clone , with --recurse-submodules
1- copied the two files
2- cd bin
2- added change_cases.sh
3- executed change_cases.sh
4- cmake -G"Unix Makefiles" .
5- make
Here's a hacky but simple Python-only solution that worked for me:
-
$ git clone --recurse-submodules https://github.com/USDAForestService/ForestVegetationSimulator.git - update
base_dirand run the following
import os
from glob import glob
base_dir = "/opt/ForestVegetationSimulator/volume/NVEL"
for f in glob(f"{base_dir}/*.[fF]"):
file_name = os.path.basename(f)
if not file_name.islower():
os.rename(f, f"{base_dir}/{file_name.lower()}")
print(f"old={file_name}, new={file_name.lower()}")
else: print(f"unchanged: {file_name}")
3.$ cd bin; cmake -G"Unix Makefiles" .
4. $ make