Adding GENDF Capabilities to ENDFtk #73

nathangibson14 · 2020-06-03T18:52:19Z

This pull request adds the ability to read a GENDF file via the syntaxTree and to do basic parsing of cross section (MFD 3) and transfer matrix (MFD 6) sections. This only supports reading GENDF files; writing is not yet implemented.

The classes of the syntaxTree are now tagged as ENDF or GENDF, and ENDF is default. This means there's no change to the existing syntax for reading ENDF files. Rather than specify the tag directly, users (and other places in the code) can use convenience aliases such as GendfTape.

Parsing is currently limited to extracting a cross section or matrix element by group, Legendre order, and dilution index. Adding capability for an array of cross sections will likely follow sooner than later.

There is plenty more to do for full GENDF support, but I believe this is enough to start using while other functionality is developed. Things to include later include:

Support for header data (MFD 1) and delayed neutron data (MFD 5)
Support for ratio quantities (MFD/MT 3/452 and various others)
Support for compressed fission transfer matrix format (prod/spec for lower energy groups)
Support for photon production
More convenient data structures from parsed data structures.

The test suite passes, although there are plenty of ranges-related test suite quirks on various systems/compilers. None of the quirks touch pieces of the code affected by this PR. I found issues with two tests on Snow/GCC 8.3.0 and one test on my MacBook/GCC 9.2.0. I'll put up a separate issue, but it's time to consider upgrading the ranges package for a variety of reasons.

Let me know how my biggest foray into C++ looks! 😄

…rom parse

…ted GendfSection.test.

Added files skipped last time.

jlconlin · 2020-06-03T19:00:10Z

src/ENDFtk/GendfDataRecord/test/GendfDataRecord.test.cpp

+      long int lineNo = 2;
+
+      THEN( "the object can be created" ) {
+        GendfDataRecord record( begin, end, lineNo, 1, 1, 9228, 3, 2 );


Is there any kind of data that someone might try to use that would be invalid? We should try and capture that and throw some kind of exception (std::exception is fine). We should also test for that here.

This is really just a wrapped ListRecord. I think most of the error handling is covered there.

jlconlin

Looks pretty good. I did not look at this for the technical correctness of a GENDF file (not familiar with it to make a judgement). I was mostly looking for C++ and style issues.

It's a good first Pull Request. Clearly you have taken after @whaeck—which isn't a bad thing, especially since ENDFtk is his project.

jlconlin · 2020-06-03T19:01:37Z

src/ENDFtk/section/Gendf3.hpp

+template<>
+class GendfType< 3 > : protected GendfBase {
+
+protected:


This doesn't need to be here with the way the class is currently configured.

jlconlin · 2020-06-03T19:03:13Z

src/ENDFtk/section/Gendf3/src/ctor.hpp

+
+  // TODO: Add support for ratio quantities
+  if( data_.at(num_groups_).NG2() == 3 ) {
+    Log::info( "ENDFtk does not yet support ratio quantities." );


If you are going to throw an exception, Log an error. You can provide informational logs afterwards as needed.

The practice we have developed is we log an error when it is initially thrown. If we catch (and possibly re-throw) an exception, then we log an informational message.

I'm not convinced this is done consistently throughout, but I'm happy to change mine. What you are suggesting makes more sense to me, I was just trying to stick to the convention I saw in similar places, which likely was non-conforming.

The way we've done it up to now is that an error is logged when the original exception is created, logging information later on to indicate where it happened, etc. by catching the exception.

jlconlin · 2020-06-03T19:06:18Z

src/ENDFtk/section/Gendf3/src/getCrossSection.hpp

+ * @param order Legendre order
+ * @param idil  Index of dilution (0-based)
+ */
+double getCrossSection( int group,


Ugh. I hate getters (or setters) that start like get... (or set...). The get is unnecessary. You can tell the difference between a getter and a setter based on the function arguments. Granted, this is a matter of opinion (mine, in fact), but it is what we have adopted for NJOY. See Section 6 of our Style Guide.

I like Python where I don't need () on my getters and setters 😄

I usually am with you. This one was because I was thinking of getCrossSectionByGroup versus getCrossSection returning the whole array. I'm pretty sure I wrote those routines into OpenMOC years ago, so they came back to me. I didn't even stick with that naming convention, so I won't argue in favor for get here!

jlconlin · 2020-06-03T19:08:22Z

src/ENDFtk/section/Gendf3/test/Gendf3.test.cpp

+std::string getSection();
+
+
+SCENARIO( "creating a GendfBase object" ) {


You are not creating a GendfBase object here. You are creating a GendfType<3>. Yeah, I'm being pedantic. I don't really care what is said here, but we should try to be accurate.

Copy and paste can be cruel. I'll fix it.

jlconlin · 2020-06-03T19:08:48Z

src/ENDFtk/section/Gendf3/test/Gendf3.test.cpp

+      HeadRecord head(begin, end, lineNo);
+
+      THEN( "the object can be created" ) {
+        section::GendfType<3> chunk(head, begin, end, lineNo, MAT );


chunk? I see you've taken after @whaeck in your naming convention

I originally had section, but felt dirty using section as both a namespace and a variable at the same time. And when I looked to other tests for guidance, voila -- I found @whaeck's approach.

Because that's what it is: a chunk of ENDF data :-p

In my Python bindings, I've been using parsec for parsed sections. I'm sure our friends in astrophysics would approve of that.

jlconlin · 2020-06-03T19:19:11Z

src/ENDFtk/section/GendfBase/src/populateData.hpp

+
+    // check if group was already seen
+    if (data_records.count( group ) > 0) {
+      Log::info( "Group {} appears multiple times in MFD/MT {}/{}",


Log::error

jlconlin · 2020-06-03T19:19:39Z

src/ENDFtk/section/GendfBase/test/GendfBase.test.cpp

+std::string invalidSEND();
+
+
+SCENARIO( "creating a GendfBase object" ) {


Here you are testing a GendfBase.

jlconlin · 2020-06-03T19:25:21Z

src/ENDFtk/syntaxTree/Section.hpp

+  /* methods */
+
+  // this file implements the parse() function
+  #include "ENDFtk/syntaxTree/Section/src/parse-gendf.hpp"


I wonder if this could be implemented with a templated parse function:

template< typename Tag > void parse< Tag >(...); template<> void parse< GENDFTag >(...){} template<> void parse< ENDFTag >(...)

This way, you don't need to specialize the class, just the parse function. It seems that everything else is the same.

I really wanted to do this, but I couldn't figure out a way to make it happen. A tag with a default value doesn't play nice with the int... OptionalMT that parse is already templated on. I also don't want the user to have to specify the tag.

If you can see a way to clean this up, I'm all for it.

In talking with Wim this morning, I think I found a cleaner solution in line with what you were saying. Stay tuned!

jlconlin · 2020-06-03T19:26:32Z

src/ENDFtk/syntaxTree/Section/test/GendfSection.test.cpp

+
+    } // WHEN
+
+    WHEN( "an invalid (MT!=0) SEND record ends the Section" ){


Good job testing failure conditions. Are there others that also need to be tested?

jlconlin · 2020-06-03T19:29:23Z

src/ENDFtk/syntaxTree/GendfMaterial.hpp

@@ -0,0 +1,71 @@
+/* Template specialization for GendfMaterial */
+template< typename BufferIterator >
+class Material< BufferIterator, GENDFTag > {


I'm not sure I see the reason for a separate GENDF Material. (Granted I didn't look real close.) It seems it is virtually identical to the Material class, just uses the GENDFTag. What exactly is the difference and can we do template specialization on a member function instead of the entire class?

There's a subtle but big difference between a GendfMaterial and a Material. A Material is processed by looping over the Files it contains, creating a map of these File objects. In a GENDF file, there is no guarantee that all sections with the same MFD are consecutive. And to make matters worse, there's no guarantee that a MFD/MT section is unique (although I'd hope the repeated section is at least identical!).

So I implemented this so that there is no GendfFile, and the GendfMaterial contains a map of GendfSection objects directly.

There are lots of places where the ENDF format is frustrating, but GENDF is much more frustrating despite having a much smaller spec!

whaeck · 2020-06-03T20:01:25Z

Stop poking me ;-)

nathangibson14 · 2020-06-03T20:06:08Z

@whaeck Poke.

whaeck

I started looking at this, there's already a few points of design we could discuss in more detail. I'll continue looking at this when I have time this week.

whaeck · 2020-06-09T04:40:33Z

src/ENDFtk/GendfDataRecord.hpp

@@ -0,0 +1,46 @@
+class GendfDataRecord : protected ListRecord {
+
+protected:


There's no need to make these members protected.

whaeck · 2020-06-09T04:43:31Z

src/ENDFtk/GendfDataRecord.hpp

+  /* getters */
+  double temperature() const { return this->C1(); };
+  int NG2() const { return this->L1(); }
+  int numSecondaryPositions() const { return NG2(); }


this->NG2()

whaeck · 2020-06-09T04:48:49Z

src/ENDFtk/GendfDataRecord.hpp

+   */
+  auto flux() const { 
+    return this->list()
+      | ranges::view::take_exactly( num_legendre_ * num_sigma0_ );


this is where you would use those public methods.

whaeck · 2020-06-09T04:51:29Z

src/ENDFtk/GendfDataRecord/src/ctor.hpp

+                 int MT )
+  : ListRecord( it, end, lineNumber, MAT, MF, MT ),
+    num_legendre_( num_legendre ),
+    num_sigma0_( num_sigma0 ) {}


You still need a constructor to create this from scratch, especially if we want to create GENDF files. It's better to add it now instead of waiting.

whaeck · 2020-06-09T13:15:07Z

src/ENDFtk/GendfDataRecord.hpp

+  int num_legendre_;
+  int num_sigma0_;


I would rename these and provide public methods to retrieve them. As for renaming, sigma0 is a bit obscure so it my be more appropriate to use dilutions instead. There's also no need to use abbreviations in these names (number instead of num).

whaeck · 2020-06-09T13:19:24Z

src/ENDFtk/GendfDataRecord.hpp

+   *        - vector cross sections (NLxNZ)
+   *        - ratio and cross sections (both NLxNZ)
+   *        - matrix elements (NLxNZxNG)


From this comment, I assume that there are essentially three types of records, which could all have their own interface? I would surmise that the matrix element one is only used in MFD6 data. Where are the other ones used?

whaeck · 2020-06-09T13:32:42Z

src/ENDFtk/section/GendfBase.hpp

+
+protected:
+  /* fields */
+  std::map< int, GendfDataRecord > data_;


What about using a template for this instead of GendfDataRecord? The GendfDataRecord can store different types of data (just like a ListRecord does). We may want to explore making specific classes that inherit from GendfDataRecord to provide a more convenient interface to their particular type of data and then using it as the template argument.

You could also add more convenience functions in this base class to see if a specific group has a record or not, etc. to avoid having to do this each GendfType class (see getCrossSection() where you verify if a group is present). By combining these together with the convenience interface for a group cross section, you could write code like this that is more compact and easier to read:

template < typename Record > class GendfBase : protected Base { std::map< unsigned int, Record > records_; public: // this gets you access to the records as if they were stored in a vector auto records() const { return ranges::view::values( this->records_ ); } // this gets you access to the a record by group index auto record( unsigned int group ) const { return this->records_ .at( group ); } // this verifies if the group is present auto hasRecord( unsigned int group ) const { return this->records_ .find( group ) != this->records_ .end(); } }; class GendfType< 3 > : protected GendfBase< GendfGroupCrossSection > { public: auto groups() { return this->records(); } auto group( unsigned int group ) { return this->record( group ); } // this gets you the full multi-group cross section // this still needs dilution, but you get the gist auto crossSection() const { return ranges::view::iota( 0, number_groups ) | ranges::view::transform( [&] ( unsigned int group ) -> double { return this->hasRecord( group ) ? this->group( group ).crossSection() : 0.0; } ); } }

nathangibson14 added 17 commits May 27, 2020 14:16

Added partial implementation of GendfTape and GendfMaterial.

b877d76

Added implementation files for GendfMaterial.

942d614

Added GendfSection.hpp

1efdb7a

Attempting to avoid separate implementation for GendfSection, aside f…

f5ab73e

…rom parse

Using BaseSection and inheritance.

2f74d15

Added tests for GendfSection, GendfMaterial. Also fixed GendfSection.

19bf9a7

Removed placeholder parse-gendf. Added testing for GendfTape and upda…

b2939c1

…ted GendfSection.test.

Added file I missed last time.

681ca66

Added implementation of GendfBase.

6022b61

Fixed GendfBase problems and added testing.

7dfe495

Moved GendfBase lists into std::map

62c0ae4

Added a GendfDataRecord class.

e40a8e1

Added files skipped last time.

Added test for GendfDataRecord.

7b279eb

Added section::Gendf3

829dd2d

Added parse-gendf.hpp to syntaxTree::Section

acfcc95

Added support for MFD6.

3f22146

Added some in-code documentation.

ca8d37f

nathangibson14 requested review from jlconlin and whaeck June 3, 2020 18:52

jlconlin reviewed Jun 3, 2020

View reviewed changes

jlconlin requested changes Jun 3, 2020

View reviewed changes

whaeck requested changes Jun 9, 2020

View reviewed changes

nathangibson14 marked this pull request as draft January 14, 2021 15:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding GENDF Capabilities to ENDFtk #73

Adding GENDF Capabilities to ENDFtk #73

nathangibson14 commented Jun 3, 2020

jlconlin Jun 3, 2020

nathangibson14 Jun 4, 2020

jlconlin left a comment

jlconlin Jun 3, 2020

jlconlin Jun 3, 2020

nathangibson14 Jun 3, 2020

whaeck Jun 3, 2020

jlconlin Jun 3, 2020

nathangibson14 Jun 3, 2020

jlconlin Jun 3, 2020

nathangibson14 Jun 3, 2020

jlconlin Jun 3, 2020

nathangibson14 Jun 3, 2020

whaeck Jun 3, 2020

nathangibson14 Jun 3, 2020

jlconlin Jun 3, 2020

jlconlin Jun 3, 2020

jlconlin Jun 3, 2020

nathangibson14 Jun 3, 2020

nathangibson14 Jun 4, 2020

jlconlin Jun 3, 2020

jlconlin Jun 3, 2020

nathangibson14 Jun 3, 2020

whaeck commented Jun 3, 2020

nathangibson14 commented Jun 3, 2020

whaeck left a comment

whaeck Jun 9, 2020

whaeck Jun 9, 2020

whaeck Jun 9, 2020

whaeck Jun 9, 2020

whaeck Jun 9, 2020

whaeck Jun 9, 2020

whaeck Jun 9, 2020

		std::string getSection();


		SCENARIO( "creating a GendfBase object" ) {

		std::string invalidSEND();


		SCENARIO( "creating a GendfBase object" ) {


		} // WHEN

		WHEN( "an invalid (MT!=0) SEND record ends the Section" ){

		@@ -0,0 +1,46 @@
		class GendfDataRecord : protected ListRecord {

		protected:

Adding GENDF Capabilities to ENDFtk #73

Are you sure you want to change the base?

Adding GENDF Capabilities to ENDFtk #73

Conversation

nathangibson14 commented Jun 3, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlconlin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whaeck commented Jun 3, 2020

nathangibson14 commented Jun 3, 2020

whaeck left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment