-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/za #191
Feature/za #191
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check my few comments and questions.
self.assertEqual( 233.0248, chunk.AWR ) | ||
self.assertEqual( 233.0248, chunk.atomic_weight_ratio ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this one not self.assertAlmostEqual for the atomic_weight_ratio (double)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be, indeed.
@@ -33,6 +33,7 @@ def verify_chunk( self, chunk ) : | |||
self.assertEqual( 152, chunk.section_number ) | |||
|
|||
self.assertAlmostEqual( 94239., chunk.ZA ) | |||
self.assertAlmostEqual( 94239., chunk.target_identifier ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't those be self.assertEqual with 94239 as integer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup
src/ENDFtk/section/15.hpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there actually a difference? Perhaps just the tabulation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is just whitespace
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with the exception of the targetIdentifier() function being added.
@@ -22,7 +22,7 @@ Isotope( ControlRecord&& cont, | |||
int MAT, | |||
int MF, | |||
int MT ) : | |||
Isotope( cont.C1(), cont.C2(), cont.L2(), | |||
Isotope( static_cast< int >( std::round( cont.C1() ) ), cont.C2(), cont.L2(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully round() does the job we talked about... int(x+0.5) should be identical
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
round goes to the closest int, which would be the same as int(x+0.5). I just trust the developers of the standard library to things correctly ;-)
src/ENDFtk/section/5.hpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
different tabulation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but also the targetIdentifier() function being added.
@@ -161,7 +161,7 @@ SCENARIO( "section::Type< 8, 459 >" ) { | |||
|
|||
WHEN( "the data is given explicitly using arrays" ) { | |||
|
|||
double zaid = 92235.; | |||
double zaid = 92235; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be an integer? Same question for the next two changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it should
All ZA identifiers are now integers across all of ENDFtk:
This will allow for easier searching on ZA values without having to do floatign point comparisons.
A human readable function alternative for ZA() was added: targetIdentifier(). Functions were added to MF6 and MF26 to search for specific reaction product identifiers (similar to the MF9 and MF10 setup).
Removed unused code and removed the setters on the basic records (HEAD, LIST, etc. are now fully immutable).