Bugfix/relative vrt

.github/workflows/build_and_test_linux.yml

@@ -1,11 +1,11 @@
-name: Build and Test flows2fim - Linux
+name: Build and Test - Linux
 
 on: [workflow_dispatch, pull_request]
 
 jobs:
   build:
     name: Build flows2fim
-    
+
     runs-on: ubuntu-latest
 
     steps:
@@ -24,7 +24,7 @@ jobs:
       run: |
         CONTAINER_ID=$(docker ps -q | head -n 1)
         if [ -n "$CONTAINER_ID" ]; then
-          docker exec $CONTAINER_ID /bin/bash -c "./scripts/build-linux-amd64.sh"  
+          docker exec $CONTAINER_ID /bin/bash -c "./scripts/build-linux-amd64.sh"
           docker compose down
         else
           echo "No running containers found"
@@ -36,7 +36,7 @@ jobs:
       with:
         name: flows2fim
         path: builds/linux-amd64
-  
+
   test:
     name: Test flows2fim
     runs-on: ubuntu-latest
@@ -45,17 +45,17 @@ jobs:
     steps:
     - name: Checkout Repository
       uses: actions/checkout@v4
-    
+
     - name: Download artifact
       uses: actions/download-artifact@v4
       with:
         name: flows2fim
         path: builds/linux-amd64
-    
+
     - name: Add flows2fim to PATH
       run: |
         ls -R builds/linux-amd64
-        sudo chmod a+x ./builds/linux-amd64/flows2fim 
+        sudo chmod a+x ./builds/linux-amd64/flows2fim
         ./builds/linux-amd64/flows2fim --version
         echo "builds/linux-amd64" >> $GITHUB_PATH
 
@@ -69,7 +69,7 @@ jobs:
       run: |
         gdalinfo --version
         which gdal_ls.py
-    
+
     - name: Issue flows2fim controls test cases
       run: |
         ./scripts/test_suite_linux.sh controls

DESIGN.md

@@ -8,7 +8,12 @@
 
 - `scripts`: Includes useful scripts for building, testing, and more.
 
-## Design Decisions
+## Design Thoughts
+
+### FIM
+1. GDAL will always use relative paths if output and inputs are in same lineage, this was the reason for removing -rel flag
+1. gdalbuildvrt don't support cloud relative paths. This does not work `gdalbuildvrt /vsis3/fimc-data/fim2d/prototype/2024_03_13/vsi_relative.vrt ./8489318/z0_0/f_1560.tif ./8490370/z0_0/f_130.tif`
+1. To simplify fim.go, all paths are converted to absolute paths and the relative logic is left to `gdalbuildvrt`
 
 ### Validate
 1. A  `-o_<type>` convention is used to allow for multiple outputs across subcommands.

README.md

@@ -1,4 +1,7 @@
 # `flows2fim`
+
+[![Build and Test - Linux](https://github.com/ar-siddiqui/flows2fim/actions/workflows/build_and_test_linux.yml/badge.svg?event=push)](https://github.com/ar-siddiqui/flows2fim/actions/workflows/build_and_test_linux.yml)
+
 ![alt text](image.png)
 ## Overview
 `flows2fim` is a command line utility program that creates composite FIMs for different flow conditions utilizing input FIM libraries and rating curves database.

cmd/fim/fim.go

@@ -14,7 +14,6 @@ import (
 var usage string = `Usage of fim:
 Given a control table and a fim library folder, create a flood inundation VRT or a merged TIFF for the control conditions.
 GDAL VSI paths can be used, given GDAL must have access to cloud creds.
-GDAL does not support relative cloud paths.
 
 FIM Library Specifications:
 - All maps should have same CRS, Resolution, vertical units (if any), and nodata value
@@ -76,11 +75,9 @@ func Run(args []string) (gdalArgs []string, err error) {
 	}
 
 	var controlsFile, fimLibDir, outputFile, outputFormat string
-	var relative bool
 
 	// Define flags using flags.StringVar
-	flags.StringVar(&fimLibDir, "lib", ".", "Directory containing FIM Library. GDAL VSI paths can be used, given GDAL must have access to cloud creds")
-	flags.BoolVar(&relative, "rel", true, "If relative paths should be used in VRT")
+	flags.StringVar(&fimLibDir, "lib", "", "Directory containing FIM Library. GDAL VSI paths can be used, given GDAL must have access to cloud creds")
 	flags.StringVar(&controlsFile, "c", "", "Path to the controls CSV file")
 	flags.StringVar(&outputFormat, "fmt", "vrt", "Output format: 'vrt', 'cog' or 'tif'")
 	flags.StringVar(&outputFile, "o", "", "Output FIM file path")
@@ -105,13 +102,7 @@ func Run(args []string) (gdalArgs []string, err error) {
 		return []string{}, fmt.Errorf("%[1]s is not available. Please install GDAL and ensure %[1]s is in your PATH", gdalCommands[outputFormat])
 	}
 
-	if strings.HasPrefix(fimLibDir, "/vsi") || strings.HasPrefix(outputFile, "/vsi") {
-		relative = false
-		// gdalbuildvrt don't support cloud relative paths
-		// this does not work gdalbuildvrt /vsis3/fimc-data/fim2d/prototype/2024_03_13/vsi_relative.vrt ./8489318/z0_0/f_1560.tif ./8490370/z0_0/f_130.tif
-	}
-
-	var absOutputPath, absFimLibPath, absOutputDir string
+	var absOutputPath, absFimLibPath string
 	if strings.HasPrefix(outputFile, "/vsi") {
 		absOutputPath = outputFile
 	} else {
@@ -120,7 +111,6 @@ func Run(args []string) (gdalArgs []string, err error) {
 			return []string{}, fmt.Errorf("error getting absolute path for output file: %v", err)
 		}
 	}
-	absOutputDir = filepath.Dir(absOutputPath)
 
 	if strings.HasPrefix(fimLibDir, "/vsi") {
 		absFimLibPath = fimLibDir
@@ -155,20 +145,13 @@ func Run(args []string) (gdalArgs []string, err error) {
 		record[2] = strings.Replace(record[2], ".", "_", -1) // Replace '.' with '_'
 		folderName := filepath.Join(absFimLibPath, reachID, fmt.Sprintf("z_%s", record[2]))
 		fileName := fmt.Sprintf("f_%s.tif", record[1])
-		filePath := filepath.Join(folderName, fileName)
-
-		if relative {
-			filePath, err = filepath.Rel(absOutputDir, filePath)
-			if err != nil {
-				return []string{}, err
-			}
-		}
+		absTifPath := filepath.Join(folderName, fileName)
 
 		// join on windows may cause \vsi
-		if strings.HasPrefix(filePath, `\vsi`) {
-			filePath = strings.ReplaceAll(filePath, `\`, "/")
+		if strings.HasPrefix(absTifPath, `\vsi`) {
+			absTifPath = strings.ReplaceAll(absTifPath, `\`, "/")
 		}
-		tifFiles = append(tifFiles, filePath)
+		tifFiles = append(tifFiles, absTifPath)
 	}
 
 	// Write file paths to a temporary file
@@ -201,9 +184,6 @@ func Run(args []string) (gdalArgs []string, err error) {
 	}
 
 	cmd := exec.Command(gdalCommands[outputFormat], gdalArgs...)
-	if !strings.HasPrefix(absOutputPath, "/vsi") {
-		cmd.Dir = absOutputDir
-	}
 	// Redirecting the output to the standard output of the Go program
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr

scripts/test_suite_linux.sh

@@ -366,7 +366,7 @@ controls_test_cases() {
 }
 
 fim_test_cases() {
-    local num_test_cases_fim=9
+    local num_test_cases_fim=8
     local failed_fim_testcases=0
     total_count=$(( total_count + num_test_cases_fim))
     # If previous directories exists, remove them
@@ -419,15 +419,10 @@ fim_test_cases() {
                 -c $controls_benchmark_dir/controls_2year.csv \
                 -fmt $format \
                 -lib $library_benchmark \
-                -o $fim_test_output_formats/$output_file \
-                -rel false &> /dev/null
+                -o $fim_test_output_formats/$output_file &> /dev/null
         done
 
     printf "(4/${num_test_cases_fim})\t>>>> Regression Tests for different output formats <<<<\n\n"
-        # Note:
-        # The vrt file creation with -rel true (default behavior) is a failing case,
-        # because the vrt contains different values for the "relativeToVRT". This is due to
-        # the behavior of the -rel flag to flows2fim fim not working as expected.
         # Capture the output of the comparison function as a variable
         diff_output=$(compare_directories "$fim_reference_output_formats" "$fim_test_output_formats" "fim")
         # If there is no difference between directories, test pass
@@ -439,26 +434,7 @@ fim_test_cases() {
             failed_fim_testcases=$((failed_fim_testcases + 1))
         fi
 
-    printf "(5/${num_test_cases_fim})\t>>>> Assert flows2fim fim -rel argument creates .vrt with correct xml output <<<<\n\n"
-        output_file=fim_2year_test_rel_false.vrt
-        $cmd fim \
-            -c $controls_benchmark_dir/controls_2year.csv \
-            -fmt "vrt" \
-            -lib $library_benchmark \
-            -o $fim_test_output_formats/$output_file \
-            -rel false &> /dev/null
-
-        diff_output=$(diff "$fim_reference_output_formats/$output_file" "$fim_test_output_formats/$output_file")
-        # If there is no differnce between files, test pass
-        if [ -z "$diff_output" ]; then
-            printf "\t \u2714 No differences in .vrt files. \n\n"
-        else
-            printf "\t \u274c Outputs differ: \n\n"
-            printf "$diff_output \n"
-            failed_fim_testcases=$((failed_fim_testcases + 1))
-        fi
-
-    printf "(6/${num_test_cases_fim})\t>>>> Assert Error thrown from no controls file parameter <<<<\n\n"
+    printf "(5/${num_test_cases_fim})\t>>>> Assert Error thrown from no controls file parameter <<<<\n\n"
         # Create and assign temp file
         tempfile=$(mktemp)
         # Test case
@@ -480,7 +456,7 @@ fim_test_cases() {
             failed_fim_testcases=$((failed_fim_testcases + 1))
         fi
 
-    printf "(7/${num_test_cases_fim})\t>>>> Assert Error thrown from no library parameter <<<<\n\n"
+    printf "(6/${num_test_cases_fim})\t>>>> Assert Error thrown from no library parameter <<<<\n\n"
         # Create and assign temp file
         tempfile=$(mktemp)
         # Test case
@@ -489,11 +465,11 @@ fim_test_cases() {
                 -fmt "tif" \
                 -o $fim_test_outputs/fim_test.tif &> "$tempfile"
         # Here we use head -n 1 to capture the first line redirected to the tmp file
-        first_line=$(tail -n 1 "$tempfile")
+        first_line=$(cat "$tempfile" | sed -n '2 p')
         # Remove temp file
         rm "$tempfile"
         # Assign error string
-        assert_expected_output_1="Error: error running gdalwarp: exit status 2"
+        assert_expected_output_1="Missing required flags"
         # Compare Error messaging and print
         if [ "$first_line" = "$assert_expected_output_1" ]; then
             printf "\t \u2714 Correct error thrown. \n\n"
@@ -502,7 +478,7 @@ fim_test_cases() {
             failed_fim_testcases=$((failed_fim_testcases + 1))
         fi
 
-    printf "(8/${num_test_cases_fim})\t>>>> Assert Error thrown from no output file parameter <<<<\n\n"
+    printf "(7/${num_test_cases_fim})\t>>>> Assert Error thrown from no output file parameter <<<<\n\n"
         # Create and assign temp file
         tempfile=$(mktemp)
         # Test case
@@ -524,7 +500,7 @@ fim_test_cases() {
             failed_fim_testcases=$((failed_fim_testcases + 1))
         fi
 
-    printf "(9/${num_test_cases_fim})\t>>>> Assert Error thrown from empty controls file <<<<\n\n"
+    printf "(8/${num_test_cases_fim})\t>>>> Assert Error thrown from empty controls file <<<<\n\n"
         # Create and assign temp file
         tempfile=$(mktemp)
         # Test case
@@ -547,7 +523,7 @@ fim_test_cases() {
             failed_fim_testcases=$((failed_fim_testcases + 1))
         fi
 
-    #printf "(10/${num_test_cases_fim})\t>>>> Test flows2fim fim pull from S3 <<<<\n\n"
+    #printf "(9/${num_test_cases_fim})\t>>>> Test flows2fim fim pull from S3 <<<<\n\n"
 
     fim_passed=$((num_test_cases_fim - failed_fim_testcases))
     total_passed=$(( total_passed + fim_passed))

testdata/reference_data/fim_output_formats/fim_2year.vrt

@@ -5,7 +5,7 @@
     <NoDataValue>-9999</NoDataValue>
     <ColorInterp>Gray</ColorInterp>
     <ComplexSource>
-      <SourceFilename relativeToVRT="0">../../reference_data/library/24274741/z_nd/f_17668.tif</SourceFilename>
+      <SourceFilename relativeToVRT="0">/home/runner/work/flows2fim/flows2fim/testdata/reference_data/library/24274741/z_nd/f_17668.tif</SourceFilename>
       <SourceBand>1</SourceBand>
       <SourceProperties RasterXSize="1185" RasterYSize="1163" DataType="Float32" BlockXSize="1185" BlockYSize="512" />
       <SrcRect xOff="0" yOff="0" xSize="1185" ySize="1163" />

testdata/test_matrix.md

@@ -16,8 +16,7 @@
 |                                                 |                      |                      |                   |
 | **fim**                                         |                      |                      |                   |
 | Generate 2,5,10,25,50,100 yr fim.tif files      | ☑              | ☑              |                   |
-| Generate fim files in different output formats  | ☑              | ☒              |                   |
-| `-rel false`                                    |                      |                      | ☒           |
+| Generate fim files in different output formats  | ☑              | ☑              |                   |
 | missing `-c`                                    |                      |                      | ☑           |
 | missing `-l`                                    |                      |                      | ☑           |
 | missing `-o`                                    |                      |                      | ☑           |