Skip to content
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/thermr #34

Merged
merged 26 commits into from
Jul 15, 2017
Merged

Feature/thermr #34

merged 26 commits into from
Jul 15, 2017

Conversation

ameliajo
Copy link
Member

THERMR input checks for all cards

"file that corresponds to an MF (endf file label) = 7 (thermal \n"
"scattering).\n"
"\n"
"nendf values are restricted to equal either 0, some integer with\n"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think a value of 0 is valid for nendf. If nothing provided, what processing can be done?

Copy link
Member Author

@ameliajo ameliajo Jul 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So test problem 1 in the manual has a value of zero, and pg. 174 of the manual states
"The required ENDF tape (nendf) is only used for MF=7 data; it can be set to zero if only free-gas scattering is needed."
Which this makes me think, I should probably set a check to ensure that if nendf == 0, then card2's iin should be equal to 1 (free gas).

}
} // THEN
} // WHEN
WHEN( "nout input is equal to nendf value" ){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about when nout is equal to nin?

REQUIRE( card1.nout.value == 22 );
} // THEN
} // WHEN
WHEN( "thermal scattering data not provided" ){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, I don't think nendf=0 is valid. If I'm wrong, please show me.

} // THEN
} // WHEN
WHEN( "an input tape is out of range" ){
iRecordStream<char> iss1( std::istringstream("20 21 100\n") );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job being thorough.

} // GIVEN

GIVEN( "invalid icoh values" ){
std::vector<int> invalidValues{-1, 4, 10, 14};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice

argument::extract< THERMR::Card3::Tempr >(iss, ntemp) );
} // THEN
} // WHEN
WHEN( "there are too many temprerature values" ){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

temprerature -> temperature

static std::string description(){
return
"The emax argument is the maximum energy for thermal treatment. For\n"
"temperatures greater than 3000 kelvin, emax and the energy grid are\n"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kelvin should be capitalized.

"linear interpolation is within a specified fractional tolerance (tol)\n"
"of the exact cross section at every point.\n"
"\n"
"tol must be some decimal value greater than 1.0e-6.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did you find this limit? It sounds reasonable, but I don't know where it comes from.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a definition of tol_min in the thermr.f90 file, on line 708, so I assumed that would be the reasonable lower bound

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very good detective work.

} // GIVEN

GIVEN( "invalid Card4 inputs" ){
WHEN( "an input tape is out of range" ){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ThisWHEN statement isn't correct.

REQUIRE_THROWS( THERMR( iss2 ) );
} // THEN
} // WHEN
WHEN( "card1 input files repeated" ){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice thoroughness.

@jlconlin
Copy link
Member

It seems there is a bug in the code and it won't compile all the way. Probably a simple fix @ameliajo

if ( not nendf.value and i != 1 ){
Log::warning("iin value indicates that the material will not be treated\n"
"as a free gas, which conflicts with earlier definition of\n"
"card1 nendf input. If nendf equals 0, iin must equal 1.\n");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

return false;
}
// Make sure temperatures are in increasing order
auto unsortedStart = std::is_sorted_until( temps.begin(), temps.end() );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

"The tempr argument is a list of ntemp temperatures (where ntemp was\n"
"defined in card2). These tempr inputs are the temperatures at which\n"
"the thermal scattering cross sections are evaluated. Each\n"
"temperature is in kelvin, and therefore must be positive.";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe mention sorted order


static bool verify( const Value_t& temps,
const Argument< THERMR::Card2::Ntemp > & ntemp ){
if ( long(temps.size()) != ntemp.value ){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The vector parser does this before the verification is called. This check is redundant.

}

// Make sure all temperatures are positive
auto found = std::find_if( temps.begin(), temps.end(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very good use of the standard algorithms

argument::extract< THERMR::Card3::Tempr >(iss, ntemp) );
} // THEN
} // WHEN
WHEN( "there are too many temperature values" ){
Copy link
Member

@apmccartney apmccartney Jul 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test ought not be done here. That check is normally done when you call to Card::clear

argument::extract< THERMR::Card3::Tempr >(iss, ntemp) );
} // THEN
} // WHEN
WHEN( "there are not enough temperature values" ){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll notice that you don't see the error message here you might have expected

}


<<<<<<< HEAD
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge needs to be resolved

@apmccartney apmccartney merged commit 071b3bd into master Jul 15, 2017
@jlconlin jlconlin deleted the feature/THERMR branch July 24, 2018 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants