diff --git a/.github/workflows/build_and_test_linux.yml b/.github/workflows/build_and_test_linux.yml index ff04d30..406d2d6 100644 --- a/.github/workflows/build_and_test_linux.yml +++ b/.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 diff --git a/DESIGN.md b/DESIGN.md index 604a01c..4d2176e 100644 --- a/DESIGN.md +++ b/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_` convention is used to allow for multiple outputs across subcommands. diff --git a/README.md b/README.md index f50cc66..ce393fd 100644 --- a/README.md +++ b/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. diff --git a/cmd/fim/fim.go b/cmd/fim/fim.go index 24680ac..f47dcdc 100644 --- a/cmd/fim/fim.go +++ b/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 diff --git a/scripts/test_suite_linux.sh b/scripts/test_suite_linux.sh index a61b84d..60580f3 100755 --- a/scripts/test_suite_linux.sh +++ b/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)) diff --git a/testdata/reference_data/fim_output_formats/fim_2year.cog b/testdata/reference_data/fim_output_formats/fim_2year.cog index f21d74b..d47f776 100644 Binary files a/testdata/reference_data/fim_output_formats/fim_2year.cog and b/testdata/reference_data/fim_output_formats/fim_2year.cog differ diff --git a/testdata/reference_data/fim_output_formats/fim_2year.vrt b/testdata/reference_data/fim_output_formats/fim_2year.vrt index 5a88b1c..14b4a04 100644 --- a/testdata/reference_data/fim_output_formats/fim_2year.vrt +++ b/testdata/reference_data/fim_output_formats/fim_2year.vrt @@ -5,7 +5,7 @@ -9999 Gray - ../../reference_data/library/24274741/z_nd/f_17668.tif + /home/runner/work/flows2fim/flows2fim/testdata/reference_data/library/24274741/z_nd/f_17668.tif 1 diff --git a/testdata/reference_data/fim_output_formats/fim_2year_test_rel_false.vrt b/testdata/reference_data/fim_output_formats/fim_2year_test_rel_false.vrt deleted file mode 100644 index 5a88b1c..0000000 --- a/testdata/reference_data/fim_output_formats/fim_2year_test_rel_false.vrt +++ /dev/null @@ -1,16 +0,0 @@ - - PROJCS["NAD83 / Conus Albers",GEOGCS["NAD83",DATUM["North_American_Datum_1983",SPHEROID["GRS 1980",6378137,298.257222101,AUTHORITY["EPSG","7019"]],AUTHORITY["EPSG","6269"]],PRIMEM["Greenwich",0,AUTHORITY["EPSG","8901"]],UNIT["degree",0.0174532925199433,AUTHORITY["EPSG","9122"]],AUTHORITY["EPSG","4269"]],PROJECTION["Albers_Conic_Equal_Area"],PARAMETER["latitude_of_center",23],PARAMETER["longitude_of_center",-96],PARAMETER["standard_parallel_1",29.5],PARAMETER["standard_parallel_2",45.5],PARAMETER["false_easting",0],PARAMETER["false_northing",0],UNIT["metre",1,AUTHORITY["EPSG","9001"]],AXIS["Easting",EAST],AXIS["Northing",NORTH],AUTHORITY["EPSG","5070"]] - -1.9071349570219149e+06, 3.0000000000000000e+00, 0.0000000000000000e+00, 3.0706380139037063e+06, 0.0000000000000000e+00, -3.0000000000000000e+00 - - -9999 - Gray - - ../../reference_data/library/24274741/z_nd/f_17668.tif - 1 - - - - -9999 - - - diff --git a/testdata/test_matrix.md b/testdata/test_matrix.md index d5a649c..97f458d 100644 --- a/testdata/test_matrix.md +++ b/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` | | | ☑ |