GEOS694 Final Project Peer Review Rubric
Instructions
- Remember this is just a class assignment, and your classmate's work. Please be respectful when providing feedback or comments.
- Open an issue in your classmate's GitHub repository and copy-paste this rubric.
- Go through the checklist, reflection questions and task below while consulting the repository.
- If any checklist items require more explanation or links to code blocks, include them.
Checklist
The code is mostly readable and follows a consistent style. The scripts are organized into classes and methods, and the method names make the workflow pretty easy to follow. I also saw a recent commit specifically for updating formatting to comply with PEP8, so it looks like formatting was considered during development.
The commit history looks strong and shows many small, specific commits. The commits are separated by task, such as adding toy problem files, adding packages to the conda environment, moving custom Canwell files into their own directory, updating README information, and making PEP8 formatting changes. This makes the development process easy to follow.
The README gives clear setup instructions using conda env create -f demDiff.yml and also gives a pip install -r requirements.txt option. I was able to create and activate the environment, but I found a syntax error when trying to run the parallel script.
The current version listed in the README is Version 2.0.0.
The repository includes a toy example with two DEMs from the Canwell Glacier area. This is useful because the example data have different CRS, vertical datums, and resolutions, so it is a realistic test case for the workflow.
The code is broken into logical steps like loading, preparing, aligning, differencing, checking stable terrain, and saving the result. The parallel script also separates sector creation, sector processing, and mosaicking, which makes the larger workflow easier to understand.
The file names are clear, especially dem_diff.py, dem_diff_parallel.py, config_template.yml, and config_toy.yml. The comments and docstrings make the main workflow understandable, and the repository structure is easy to navigate.
The README explains what the project does, why it was made, and how to run both the basic and parallel versions. The example commands are helpful, although one command using --workers auto does not match the current parser behavior because the code expects an integer.
Task #1 appears to be the class-based implementation. The main workflow is organized into classes like DEMDifferencer and DEMDifferencerParallel.
Task #2 appears to be the parameter input system. The project uses YAML config files so users can change DEM paths, CRS information, nodata values, and options without editing the main Python scripts.
Reflection Questions
1. What does this code do?
This code compares two DEM files that may have different resolutions, CRS, or vertical datums. It lines them up properly and outputs a new raster showing the elevation difference between DEM2 and DEM1.
2. How does this code do it?
It reads the input paths and CRS info from a YAML config file, loads both DEMs with xdem, converts them to a common CRS/datum, aligns them to the same grid, and then subtracts one from the other. The parallel version splits the DEM into sectors so parts of the differencing can run at the same time.
3. What data does this code work with?
It works with raster DEM data, mainly GeoTIFF files. The example dataset uses a 2010 IFSAR DEM and a 2025 lidar DEM from the Canwell Glacier area.
4. What do you find cool/interesting/unique about this code?
I thought it was interesting that the code handles the messy parts of DEM differencing, like different CRS, vertical datums, and pixel sizes, instead of just subtracting the rasters directly. The stable-terrain check also seems useful because it gives a quick way to see if there might still be a vertical offset.
5. How can this code be improved?
The main improvement I noticed is fixing small run issues, like the f-string syntax error in dem_diff_parallel.py and the mismatch between the README command using --workers auto and the code expecting an integer. I would also suggest using os.path.join() for output paths so the code is less dependent on trailing slashes.
Task: Pull Request
- Find an error/typo/bug/inconsistency in the code, it can be really minor, this is just to practice making a pull request.
I found a syntax error in dem_diff_parallel.py inside the load_config function. When I tried running the parallel script with:
python dem_diff_parallel.py config_toy.yml --num-sectors 4
the script stopped before running because two f-strings had incorrect quotation marks.
The error was in this section:
raise ValueError(f"Config missing required section: "{section}"")and also in the following required-field error message.
- Fork your classmate's repository and clone the repo to your own computer.
Completed. I forked the repository and cloned it to my local computer.
- Fix the error that you found on your local version and push the change to your forked repository.
Completed. I created a new branch called fix-parallel-fstring-syntax, fixed the f-string quotation issue in dem_diff_parallel.py, committed the change, and pushed the branch to my fork.
- Open a pull request in your classmate's repository requesting the change you made be incorporated into their repository.
Completed. I opened a pull request from my fork/branch into the original repository.
- In the pull request, write a small message describing the change, and any additional information you'd like to include.
Completed. In the pull request, I explained that the change fixes a syntax error in dem_diff_parallel.py so the parallel script can get past the config validation step.
- Link to your pull request in this issue, and link to this issue in your pull request.
Pull request link: #4
GEOS694 Final Project Peer Review Rubric
Instructions
Checklist
The code is mostly readable and follows a consistent style. The scripts are organized into classes and methods, and the method names make the workflow pretty easy to follow. I also saw a recent commit specifically for updating formatting to comply with PEP8, so it looks like formatting was considered during development.
The commit history looks strong and shows many small, specific commits. The commits are separated by task, such as adding toy problem files, adding packages to the conda environment, moving custom Canwell files into their own directory, updating README information, and making PEP8 formatting changes. This makes the development process easy to follow.
The README gives clear setup instructions using
conda env create -f demDiff.ymland also gives apip install -r requirements.txtoption. I was able to create and activate the environment, but I found a syntax error when trying to run the parallel script.The current version listed in the README is Version 2.0.0.
The repository includes a toy example with two DEMs from the Canwell Glacier area. This is useful because the example data have different CRS, vertical datums, and resolutions, so it is a realistic test case for the workflow.
The code is broken into logical steps like loading, preparing, aligning, differencing, checking stable terrain, and saving the result. The parallel script also separates sector creation, sector processing, and mosaicking, which makes the larger workflow easier to understand.
The file names are clear, especially
dem_diff.py,dem_diff_parallel.py,config_template.yml, andconfig_toy.yml. The comments and docstrings make the main workflow understandable, and the repository structure is easy to navigate.The README explains what the project does, why it was made, and how to run both the basic and parallel versions. The example commands are helpful, although one command using
--workers autodoes not match the current parser behavior because the code expects an integer.Task #1 appears to be the class-based implementation. The main workflow is organized into classes like
DEMDifferencerandDEMDifferencerParallel.Task #2 appears to be the parameter input system. The project uses YAML config files so users can change DEM paths, CRS information, nodata values, and options without editing the main Python scripts.
Reflection Questions
1. What does this code do?
This code compares two DEM files that may have different resolutions, CRS, or vertical datums. It lines them up properly and outputs a new raster showing the elevation difference between DEM2 and DEM1.
2. How does this code do it?
It reads the input paths and CRS info from a YAML config file, loads both DEMs with
xdem, converts them to a common CRS/datum, aligns them to the same grid, and then subtracts one from the other. The parallel version splits the DEM into sectors so parts of the differencing can run at the same time.3. What data does this code work with?
It works with raster DEM data, mainly GeoTIFF files. The example dataset uses a 2010 IFSAR DEM and a 2025 lidar DEM from the Canwell Glacier area.
4. What do you find cool/interesting/unique about this code?
I thought it was interesting that the code handles the messy parts of DEM differencing, like different CRS, vertical datums, and pixel sizes, instead of just subtracting the rasters directly. The stable-terrain check also seems useful because it gives a quick way to see if there might still be a vertical offset.
5. How can this code be improved?
The main improvement I noticed is fixing small run issues, like the f-string syntax error in
dem_diff_parallel.pyand the mismatch between the README command using--workers autoand the code expecting an integer. I would also suggest usingos.path.join()for output paths so the code is less dependent on trailing slashes.Task: Pull Request
I found a syntax error in
dem_diff_parallel.pyinside theload_configfunction. When I tried running the parallel script with:the script stopped before running because two f-strings had incorrect quotation marks.
The error was in this section:
and also in the following required-field error message.
Completed. I forked the repository and cloned it to my local computer.
Completed. I created a new branch called
fix-parallel-fstring-syntax, fixed the f-string quotation issue indem_diff_parallel.py, committed the change, and pushed the branch to my fork.Completed. I opened a pull request from my fork/branch into the original repository.
Completed. In the pull request, I explained that the change fixes a syntax error in
dem_diff_parallel.pyso the parallel script can get past the config validation step.Pull request link: #4