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

Refactoring trait pipeline (1/2) #45

Closed
wants to merge 72 commits into from
Closed

Conversation

linwang9926
Copy link
Collaborator

@linwang9926 linwang9926 commented Jul 20, 2023

  • Reorganizing the trait_map dictionary in the format: "trait_name": (function, ["input_trait1", "input_trait2", ...], {"additional_kwarg1": True, "additional_kwarg2": 0.5})
  • modify ellipse, network functions by adding an Optional argument(s). Take ellipse for example:
    • def get_ellipse_a(pts_all_array: np.ndarray,ellipse: Optional[Tuple[float, float, float]],).
    • The argument "ellipse" is ellipse feature, the values will be saved to the traits dictionary once we did trait calculation and BFS for the edges (traits).

==================================================================================

  • Update ellipse and network functions argument. If the arguments have different types (Tuple vs. np.ndarray), then use a Union to make this arguments inputting as pts_all_array or ellipse Tuple. If they are the same type, them make two arguments. for example:
    • get_ellipse_a(pts_all_array: Union[np.ndarray, Tuple[float, float, float]])
    • get_network_solidity(primary_pts: np.ndarray,lateral_pts: np.ndarray,
      chull_area: float = None,pts_all_array: Optional[np.ndarray] = None,monocots: bool = False,)
  • Modify the ellipse and network test functions
  • Reorganize the arguments in trait_map dictionary in get_traits_value_frame function.

==================================================================================

  • Update get_root_angle function by adding node_ind as an arugment.
  • Update base-related functions (get_base_ct_density, get_base_length_ratio) by assuming the primary_pts as primary_length_max.
  • Update get_base_median_ratio function by using lateral_base_ys and primary_tip_pt_y.
  • Modify argument pts as Optional in get_base_tip_dist and get_grav_index functions
  • Draft the trait_definitions using the defined TraitDef class
  • Redo the get_node_ind function:
    • if proximal, get the nearest non-Nan node index.
    • If proximal, just use the first half root (if odd number node, the centre node is included).
    • If not proximal, just use the last half root (if odd number node, the centre node is included).
    • if there is no non-Nan node detected, just return an index of 0 (base node).

Lin Wang and others added 28 commits July 20, 2023 15:31
…se graph and add comments. Delete duplicate `primary_depth`.
@eberrigan eberrigan changed the title Reorganizing the trait_map and modify ellipse, network functions. Refactoring trait pipeline (1/2) Jul 28, 2023
"""Find angles for each root.

Args:
pts: Numpy array of points of shape (instances, nodes, 2).
node_ind: primary or lateral root node index.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Uppercase "P" in primary :)

Copy link
Collaborator

@eberrigan eberrigan left a comment

Choose a reason for hiding this comment

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

looks good!

@@ -24,15 +25,17 @@ def get_bases(pts: np.ndarray, monocots: bool = False) -> np.ndarray:


def get_base_tip_dist(
Copy link
Collaborator

Choose a reason for hiding this comment

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

the input traits always have to match the trait definition. This function is too complicated. We should simplify it to just assume taking in the expected input traits from from the trait graph.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The base_pts and tip_pts can be used for our pipeline to quickly calculate the base-tip distance. The optional pts is designed for general purpose.

@@ -107,7 +108,7 @@ def get_root_lengths_max(pts: np.ndarray) -> np.ndarray:
def get_grav_index(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here: only input traits in the trait definition can be arguments to the function.

@linwang9926
Copy link
Collaborator Author

I redo the get_node_ind function:

  • if proximal, get the nearest non-Nan node index.
  • If proximal, just use the first half root (if odd number node, the centre node is included).
  • If not proximal, just use the last half root (if odd number node, the centre node is included).
  • if there is no non-Nan node detected, just return an index of 0 (base node).

@linwang9926
Copy link
Collaborator Author

linwang9926 commented Aug 4, 2023

Network-related functions need further test after testing get_root_lengths function.

@@ -41,18 +41,36 @@ def get_node_ind(pts: np.ndarray, proximal: bool = True) -> np.ndarray:
return np.nan

if proximal:
Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry I tried to vectorize this but looks like I did it incorrectly. If it was better before you can change it back!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No problem! The current version also works and uses vectors instead of a for loop.

@talmo talmo closed this Aug 17, 2023
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.

3 participants