Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Forge Algorithms #30

Open
wants to merge 45 commits into
base: develop
Choose a base branch
from
Open

New Forge Algorithms #30

wants to merge 45 commits into from

Conversation

sliu008
Copy link
Contributor

@sliu008 sliu008 commented Nov 18, 2024

  • Added a new bin average method for thinning
  • Added open cv strategy to generating footprint
  • Added License
  • Update way configuration structure are for forge-py
  • Update documentations

README.md Outdated
"simplify":0.3,
"min_area": 30,
"fill_kernel": [30,30],
"group": "group for lon lat"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's get rid of these groups and put them in the latVar, lonVar, and timeVar.
Same for in alpha_shape

]
"footprint":{
"strategy":"alpha_shape",
"alpha_shape":{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update this example file


wkt_alphashape = dumps(alpha_shape)
return wkt_alphashape
return dumps(footprint, trim=True)


def fit_footprint(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only for alpha_shape right? Can you move all the alpha_shape strategy to a new file? And keep the forge.py small and simple. Maybe you can create a new subdir with strategy and then put the open_cv_footprint.py and the alpha_shape_footprint.py in there to keep it more organized. Sound good?

@@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]

### Added
- Updated alpha shape algorithm with bin average thinning
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update this with the new suggestions too

README.md Outdated
## Configuration Options

### `footprint`
* **`lonVar`** (string, required): Longitude variable in the dataset.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add that group is supported and give an example

README.md Outdated
* **`pixel_height`** (int, optional, default: 1800): Desired pixel height for the input image.
* **`min_area`** (int, optional): Minimum area for polygons to be retained.
* **`fill_kernel`** (list of int, optional, default: [20,20]): Kernel size for filling holes in polygons.
* **`group`** (string, optional): NetCDF file group to use for input data for lon lat.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this

README.md Outdated
* **`value`** (list of float or float): Thinning parameters.
* **`cutoff_lat`** (int, optional): Latitude cutoff for smoothing.
* **`smooth_poles`** (list of int, optional): Latitude range for smoothing near poles.
* **`group`** (string, optional): NetCDF file group to use for input data for lon lat.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this

README.md Outdated
"thinning": {"method": "bin_avg", "value": [0.5, 0.5]},
"cutoff_lat": 80,
"smooth_poles": [78,80],
"group": "group for lon lat",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove

return strategy, {**common_params, **strategy_params}


def thinning_bin_avg(x, y, rx, ry):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to their specific file in the strategy, e.g. alpha_shape

return xy_thinned["x"].values, xy_thinned["y"].values


def remove_small_polygons(geometry, min_area):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to open_cv file, unless it's used by both? is it?

"strategy":"alpha_shape",
"alpha_shape":{
"thinning":{
"method":"standard",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs updating
"thinning": {"method": "bin_avg", "value": [0.5, 0.5]},

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still needs updating

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it work with one value and not an array?

podaac/forge_py/forge.py Outdated Show resolved Hide resolved
podaac/forge_py/forge.py Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
"strategy":"alpha_shape",
"alpha_shape":{
"thinning":{
"method":"standard",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still needs updating

"strategy":"alpha_shape",
"alpha_shape":{
"thinning":{
"method":"standard",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it work with one value and not an array?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants