search

svalinn / dagmc_stats

Tool for calculating and reporting statistics about DAGMC models
BSD 3-Clause "New" or "Revised" License
1 stars 5 forks source link

Funtion for triangle area calculation in dagmc_stats.py file and its partial test #54

Closed yqin43 closed 4 years ago

yqin43 commented 4 years ago

Description

method for triangle area calculation in dagmc_stats.py file and its partial test

Motivation and Context

calculate the areas of triangles

Changes

In dagmc_stats.py: added function for triangle area calculation(get_area_triangle) In generate_stats.py: added code to call the get_area_triangle function In test_dagmc_stats.py: added partial test to test the get_area_triangle function. Tests if the number of areas generated is equal to the number of triangles.

yqin43 commented 4 years ago

Hi everyone,

I have updated the code and please have a look! Thank you!

I have edited my code based on the comments except combing two functions to one. I find that there are many other things to change in the generate_stats.py file corresponding to combing the two functions so I prefer not to combine the two functions as it is pretty complex. Thank you!

gonuke commented 4 years ago

I don't think the suggestion was to combine two functions into one. Rather, I think the suggestion is make 2 functions into 3.

Perhaps an easy first step will be to write a function that calculates the area of a triangle given three coordinates. Even if it wasn't used in any other function, it would dramatically help with testing in the current function.

kkiesling commented 4 years ago

Thanks for making these changes @yqin43 - we'll talk in person about how we can refactor what we have according to my and @gonuke's comments.

yqin43 commented 4 years ago

Hi everyone, I made the edits and please have a look! Thank you!

kkiesling commented 4 years ago

Hi everyone, I made the edits and please have a look! Thank you!

@yqin43 I don't see any updates to making the get_tri_area(side_lengths) and get_tri_aspect_ratio(side_lengths) functions? Did you make those changes too?

kkiesling commented 4 years ago

Hi everyone, I made the edits and please have a look! Thank you!

@yqin43 I don't see any updates to making the get_tri_area(side_lengths) and get_tri_aspect_ratio(side_lengths) functions? Did you make those changes too?

Never mind! I see where these changes were made. Sorry for the delay.

This looks good to me now! I'll let @gonuke approve before merging.

gonuke commented 4 years ago

This PR is looking good now. A lot of important design elements have been addressed and the structure is a great starting place and model for going forward. Thanks for your work @yqin43