488 refactor energy #506

yunfanzhou · 2023-11-09T15:38:03Z

issue 488 refactor energy

…lkit into 488-refactor-energy

…eFunction, DIfferentiableFunction to have arguments of Simplex type instead of Tuple type

…ion to use domain simplex instead of tuple

…n. get_grad, get_hessian takes domain_simplex and variable_simplex as arguments instead of tuples

…ray of DScalar type vectors

…ain to obtain 3 vertices or edges for Autodiff vector

codecov · 2023-11-09T19:37:21Z

Codecov Report

Attention: 132 lines in your changes are missing coverage. Please review.

Comparison is base (435ae49) 83.35% compared to head (6e582ae) 85.27%.
Report is 88 commits behind head on main.

Files	Patch %	Lines
src/wmtk/io/MshReader.cpp	81.52%	17 Missing ⚠️
src/wmtk/simplex/cofaces_single_dimension.cpp	30.43%	16 Missing ⚠️
src/wmtk/function/TriangleAutodiffFunction.cpp	65.71%	12 Missing ⚠️
src/wmtk/io/HDF5Reader.cpp	77.77%	12 Missing ⚠️
components/wmtk_components/output/output.cpp	0.00%	10 Missing ⚠️
src/wmtk/function/AMIPS.cpp	58.82%	7 Missing ⚠️
...mtk/multimesh/utils/find_local_switch_sequence.cpp	0.00%	7 Missing ⚠️
src/wmtk/simplex/top_dimension_cofaces.cpp	33.33%	6 Missing ⚠️
...onents/isotropic_remeshing/isotropic_remeshing.cpp	0.00%	5 Missing ⚠️
components/wmtk_components/mesh_info/mesh_info.cpp	0.00%	5 Missing ⚠️
... and 15 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #506      +/-   ##
==========================================
+ Coverage   83.35%   85.27%   +1.92%     
==========================================
  Files         215      207       -8     
  Lines        5954     6032      +78     
==========================================
+ Hits         4963     5144     +181     
+ Misses        991      888     -103

Flag	Coverage Δ
wildmeshing	`85.27% <69.15%> (+1.92%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

mtao

fix the simplex stuff and try to add some more unit tests, but other than that looks good. i'l'l leave this at approve, but please try to improve the code coverage.

src/wmtk/function/AutodiffFunction.hpp

src/wmtk/simplex/cofaces_single_dimension.hpp

src/wmtk/simplex/cofaces_single_dimension.cpp

mtao · 2023-11-13T12:23:46Z

Undo last changes to the simplex folder. This undoes an internal API practice

daniel-zint

Main concerns: documentation and coverage. The code itself looks good to me but especially with the simplex stuff, we should really make sure that it is 100% covered because everyone is going to use that.

src/wmtk/function/AMIPS.hpp

src/wmtk/function/AMIPS.cpp

daniel-zint · 2023-11-13T21:32:39Z

src/wmtk/function/AMIPS.cpp

+        throw std::runtime_error("AMIPS only supports 2D and 3D meshes");
+    }
+    if (embedded_dimension() == 2) {
+        DSVec2 coordinate0_2d = coordinate0;


Is that conversion safe? It seems to me like something that at least throws a warning and should be guarded properly by asserts). Actually, it might be the best if all the coordinates use a templated type.

daniel-zint · 2023-11-13T21:33:24Z

src/wmtk/function/AMIPS.hpp

+    ~AMIPS();
+
+protected:
+    DScalar eval(DSVec& coordinate0, DSVec& coordinate1, DSVec& coordinate2) const override;


I think all the parameters should be const.

And where are DSVec and DScalar defined? I couldn't find it in autodiff.h.

DSVec and DScalar are defined in my AutodiffFunction.

src/wmtk/function/AutodiffFunction.cpp

src/wmtk/function/utils/autodiff.h

src/wmtk/operations/tri_mesh/internal/VertexSmoothGradientDescent.cpp

daniel-zint · 2023-11-13T22:58:45Z

src/wmtk/simplex/cofaces_single_dimension.hpp


-std::vector<Tuple> cofaces_single_dimension_tuples(
+SimplexCollection cofaces_single_dimension(


This one definitely needs documentation.

I'm going to leave everything that's related to cofaces_single_dimension unchanged for now, since Michael is going to possibly rewrite a lot about this function in #514

daniel-zint · 2023-11-13T23:00:20Z

src/wmtk/simplex/cofaces_single_dimension.hpp

@@ -17,4 +18,10 @@ std::vector<Tuple> cofaces_single_dimension_tuples(
    const Mesh& mesh,
    const Simplex& my_simplex,
    PrimitiveType cofaces_type);
+
+std::vector<Simplex> cofaces_single_dimension_simplices(


Getting the vector of simplices from the simplex collection is very simple. I don't think this needs its own function.

daniel-zint · 2023-11-13T23:01:39Z

src/wmtk/simplex/cofaces_single_dimension.cpp

-std::vector<Tuple> cofaces_single_dimension_tuples(
-    const TriMesh& mesh,
-    const Simplex& my_simplex,
+std::vector<Simplex> cofaces_single_dimension_simplices(


This is a function that is going to be used by everyone. It should have 100% coverage.

daniel-zint

lgtm

yunfanzhou added 9 commits November 2, 2023 14:09

Merge branch 'main' of https://github.com/wildmeshing/wildmeshing-too…

0337c4a

…lkit into 488-refactor-energy

changing all functions in Function, LocalFunction, LocalDifferentiabl…

adc4bcc

…eFunction, DIfferentiableFunction to have arguments of Simplex type instead of Tuple type

adding coaces_single_dimension_simplices to helpw with Function class

b60cb57

delete PerSimplexClass dependency on Function. change get_value_funct…

4fba2a5

…ion to use domain simplex instead of tuple

PerSimplexDifferentiableFunction only inherits from PerSimplexFunctio…

d6cbda4

…n. get_grad, get_hessian takes domain_simplex and variable_simplex as arguments instead of tuples

implemented the function get_variable_coordinates which returns an ar…

8d52592

…ray of DScalar type vectors

add TriangleAutodiffFunction. Implement navigations in a triangle dom…

fc03e75

…ain to obtain 3 vertices or edges for Autodiff vector

AMIPS is now just evoking the utils energy implementation

069faa6

change the energy application in component accordingly

44e61a4

yunfanzhou linked an issue Nov 9, 2023 that may be closed by this pull request

refactor energy #488

Closed

7 tasks

yunfanzhou requested review from mtao and daniel-zint November 9, 2023 15:38

autodiff scop set at the top level of the energy eval

727aa93

mtao approved these changes Nov 9, 2023

View reviewed changes

yunfanzhou added 2 commits November 12, 2023 21:37

delete resizing

013c904

change cofaces single dimension

00ad44d

yunfanzhou self-assigned this Nov 13, 2023

daniel-zint requested changes Nov 13, 2023

View reviewed changes

changes based on Daniel's review

6e582ae

daniel-zint approved these changes Nov 14, 2023

View reviewed changes

yunfanzhou merged commit ae84aa8 into main Nov 14, 2023

yunfanzhou deleted the 488-refactor-energy branch November 14, 2023 19:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

488 refactor energy #506

488 refactor energy #506

yunfanzhou commented Nov 9, 2023

codecov bot commented Nov 9, 2023 •

edited

Loading

mtao left a comment

mtao commented Nov 13, 2023

daniel-zint left a comment

daniel-zint Nov 13, 2023

daniel-zint Nov 13, 2023

yunfanzhou Nov 14, 2023

daniel-zint Nov 13, 2023

yunfanzhou Nov 14, 2023

daniel-zint Nov 13, 2023

daniel-zint Nov 13, 2023

daniel-zint left a comment


		std::vector<Tuple> cofaces_single_dimension_tuples(
		SimplexCollection cofaces_single_dimension(

488 refactor energy #506

488 refactor energy #506

Conversation

yunfanzhou commented Nov 9, 2023

codecov bot commented Nov 9, 2023 • edited Loading

Codecov Report

mtao left a comment

Choose a reason for hiding this comment

mtao commented Nov 13, 2023

daniel-zint left a comment

Choose a reason for hiding this comment

daniel-zint Nov 13, 2023

Choose a reason for hiding this comment

daniel-zint Nov 13, 2023

Choose a reason for hiding this comment

yunfanzhou Nov 14, 2023

Choose a reason for hiding this comment

daniel-zint Nov 13, 2023

Choose a reason for hiding this comment

yunfanzhou Nov 14, 2023

Choose a reason for hiding this comment

daniel-zint Nov 13, 2023

Choose a reason for hiding this comment

daniel-zint Nov 13, 2023

Choose a reason for hiding this comment

daniel-zint left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 9, 2023 •

edited

Loading