Protocol buffer implementation #105
Conversation
…t, added delete for gcda files
…antiated Materials
JessicaRehak
added
enhancement
|
Will do it later today or tomorrow morning
On Tue, Oct 2, 2018 at 10:31 Joshua Rehak ***@***.***> wrote:
@jsrehak <https://github.com/jsrehak> requested your review on:
SlaybaughLab/BART#105 <#105>
Protocol buffer implementation.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#105 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AajDlHIJmwrNqQD-OXn7TWq_sbTeZqpBks5ug6LigaJpZM4XEg-0>
.
--
Weixiong Zheng, Postdoctoral Scholar
Emails: [email protected]; [email protected]
Tel: (979)676-3199
UC Berkeley
|
Codecov Report
@@ Coverage Diff @@
## restart #105 +/- ##
============================================
+ Coverage 24.78% 37.26% +12.47%
============================================
Files 27 31 +4
Lines 2017 2128 +111
============================================
+ Hits 500 793 +293
+ Misses 1517 1335 -182
Continue to review full report at Codecov.
|
src/material/material_properties_I.h
Outdated
| #include <vector> | ||
|
|
||
| #include <deal.II/lac/full_matrix.h> | ||
|
|
There was a problem hiding this comment.
Please explain a bit:
- why we need this base class for MaterialProperties
- What "I" means for the naming. The naming should somehow self-explain what it does.
- Please add the brief doxygen documentation for this class
| }; | ||
|
|
||
| TEST_F(TestHelperFunctionTest, RandomDoubleTest) { | ||
|
|
| TEST_F(TestHelperFunctionTest, RandomIntMatrixMapTest) { | ||
| // Generate 20 random int to matrix maps and verify properties | ||
| for (int i = 0; i < 20; ++i) { | ||
| size_t n = rand()%10 + 1; |
There was a problem hiding this comment.
size_t and rand are objects in std namespace, so please specify the namespace accordingly.
|
|
||
| double val1 = -200 + static_cast<double>(rand()) / RAND_MAX*(400); | ||
| double val2 = -200 + static_cast<double>(rand()) / RAND_MAX*(400); | ||
| double min = (val1 > val2) ? val2 : val1; |
There was a problem hiding this comment.
Rename min and max to something else such as min_val and max_val. We should avoid using names that means something in std namespace.
| test_chi_[10] = {0.5925247816749882, 0.4071432856463152, 0.00033193267869671705, 0, 0, 0, 0}; | ||
| test_chi_[11] = {0.5925247816749882, 0.4071432856463152, 0.00033193267869671705, 0, 0, 0, 0}; | ||
| test_sigma_s_[1] = { | ||
| 0.12334, 0.055604999999999995, 0.00023375, 0, 0, 0, 0, |
There was a problem hiding this comment.
Two more spaces for breaking-line indent. Adjust all applicable spots.
| } | ||
|
|
||
| TEST_F(MaterialPropertiesTest, WrongNumberOfMaterials) { | ||
| EXPECT_THROW({ |
There was a problem hiding this comment.
The format is actually messy for all the EXPECT_THROW macros have try and catch in it. Remember, breaking a line in parentheses should follow one of the following two rules:
- all new lines should at least look up the open parenthesis (left)
- all new lines should based on indenting with 4 spaces.
Please change accordingly. Also pls read Google Style guide for this painful rule or discuss with me.
| std::unordered_set<int> material_ids = {1, 2, 10, 11}; | ||
| std::unordered_map<int, bool> correct_fissile_id_map = {{1, false}, {2, false}, {10, false}, {11, false}}; | ||
| std::unordered_map<int, std::vector<double>> zero_vals_map = | ||
| {{1, {0, 0, 0, 0, 0, 0, 0}}, {2 |
| std::unordered_set<int> material_ids = {1, 2, 10, 11}; | ||
| std::unordered_map<int, bool> correct_fissile_id_map = {{1, false}, {2, false}, {10, false}, {11, false}}; | ||
| std::unordered_map<int, std::vector<double>> zero_vals_map = | ||
| {{1, {0, 0, 0, 0, 0, 0, 0}}, {2 |
src/material/material_properties.h
Outdated
| argument data types, | ||
| and output sequence for print_info() | ||
| */ | ||
|
|
There was a problem hiding this comment.
One comment: if dealii:: namespace is applicable to the macros, we should apply it (we don't if not)
One thing to change: add two more spaces for indent. Read Google Style Guide
|
The No changes to fix formatting in the |
This replaces the
Materialsclass withMaterialPropertiesthat ingests a material file in the form of a protocol buffer (proto file provided) or a plain text file in the correct formatting.Detailed list of changes:
MaterialPropertiesthat ingests a provided material file and provides checks for data consistency, and access to the data.MaterialPropertiestesting.-rflag tocoverage.shthat will generate an html coverage report.material.proto, the protocol buffer file used for materials.BuildMaterialfunctions fromBartBuilderbecause all material information is now through the interface of aFundamentalDataobjectXSectionsstruct to use newMaterialPropertiesobject, implemented testing for the struct.MaterialPropertiesIinterface abstract class.MockMaterialPropertiesfor testing without instantiating aMaterialPropertiesclass; various helpful test functions for making random vectors,FullMatrixobjects, and maps that map those objects to integers; various testing material protocol buffer files.PreciseSumthat uses the Neumaier variation of the Kahan summation.