search

imcs-compsim / MIRCO

A shared-memory parallel BEM code for the contact of rough surfaces
MIT License
2 stars 3 forks source link

Refactor code #44

Closed RShaw026 closed 2 years ago

RShaw026 commented 2 years ago

Description and Context

The evaluate function was very big. It has been refactored with various small functions. This will also help in writing the unit tests for every function.

A minor change has been made in one of the input files. While migrating from json input files to xml, the tolerance value was copied incorrectly from 0.01 to just 0.0 which was causing a different output. That has been fixed and now the results are same as before.

Related Issues and Pull Requests

How Has This Been Tested?

the already existing input files are giving the same results as before.

Checklist

Additional Information

Interested Parties / Possible Reviewers

@NoraHagmeyer @mayrmt

mayrmt commented 2 years ago

@RShaw026 I totally see the value in splitting a long function into several small functions. However, I don't think, that every function has to live in its own set of header/cpp-file. You can group multiple functions into one file. If there are sets of functions, that are related to each other within a set, but the sets are distinct, then maybe group all functions of a related set into one file and put the functions of an unrelated set into another file.

RShaw026 commented 2 years ago

@NoraHagmeyer I have addressed the comments. Please review again.

NoraHagmeyer commented 2 years ago

Seems fine to me! I took the liberty of starting the tests again. If everything still works the way it's supposed to we'll only know once you've written unittests for the newly created functions. Merge ahead.

Seems that manually started checks are not shown here. But they passed: https://github.com/imcs-compsim/MIRCO/actions/runs/1881808348