Add option to convert cooling times in output file to seconds #148
Conversation
|
There is a merge conflict to be resolved here thanks to #215 |
Merge modified code with PR branch
|
I've pulled from this branch and tested out these changes, and they appear to be working as expected. Some documentation changes are likely still needed. |
gonuke
left a comment
gonuke
left a comment
There was a problem hiding this comment.
I'm a little surprised that this compiles, given some mismatches in the header and code.
| output information can be given. */ | ||
| void Loading::write(int response, int writeComp, CoolingTime* coolList, | ||
| int targetKza, int normType) | ||
| int targetKza, int normType, const OutputFormat* outFmt) |
There was a problem hiding this comment.
Why do you need outFmt here? it doesn't appear to be used
And if you don't need it, then you don't need to include it's header above.
| output information can be given. */ | ||
| void Mixture::write(int response, int writeComp, CoolingTime* coolList, | ||
| int targetKza, int normType) | ||
| int targetKza, int normType, const OutputFormat* outFmt) |
There was a problem hiding this comment.
Why do you need outFmt here? it doesn't appear to be used.
And if you don't need it, then you don't need to include it's header above.
| } | ||
|
|
||
| void CoolingTime::getCoolTimesStrings(std::vector<std::string>& coolTimesList) | ||
| void CoolingTime::getCoolTimesStrings(std::vector<std::string>& coolTimesList, const OutputFormat* outFmt) |
There was a problem hiding this comment.
The header defines this as an integer not a a pointer to type OutputFormat. I think I like that choice better since it means you don't need to test if outFmt is defined below.
| cout << type; | ||
| if (strlen(type) < 8) | ||
| cout << "\t"; | ||
| cout << " shutdown "; |
There was a problem hiding this comment.
Does this need to include the half-life column now as well?
| next->cooltimeType = COOLTIME_DEF; | ||
| break; |
There was a problem hiding this comment.
Thanks to a quirk of C++ you don't need these two lines
| next->cooltimeType = COOLTIME_DEF; | |
| break; |
| break; | ||
| case (OUTFMT_CDOSE) : | ||
| sprintf(buffer,Out_Types_Str[outTypeNum], | ||
| ptr->contactDose->getFileName()); |
There was a problem hiding this comment.
I think we need to write the cooling time units for these ones, too, probably
| can be given. */ | ||
| void Volume::write(int response, int writeComp, CoolingTime* coolList, | ||
| int targetKza, int normType) | ||
| int targetKza, int normType, const OutputFormat* outFmt) |
There was a problem hiding this comment.
Why do you need outFmt here? it doesn't appear to be used.
And if you don't need it, then you don't need to include it's header above.
| cooltime_units = 1; | ||
| break; |
There was a problem hiding this comment.
Don't need these lines since the same thing happens in both cases
| cooltime_units = 1; | |
| break; |
| switch (cooltimeType) { | ||
| case COOLTIME_S: | ||
| cooltime_units = 2; | ||
| break; | ||
| case COOLTIME_DEF: | ||
| cooltime_units = 1; | ||
| break; | ||
| default: | ||
| cooltime_units = 1; | ||
| } |
There was a problem hiding this comment.
Why not just:
| switch (cooltimeType) { | |
| case COOLTIME_S: | |
| cooltime_units = 2; | |
| break; | |
| case COOLTIME_DEF: | |
| cooltime_units = 1; | |
| break; | |
| default: | |
| cooltime_units = 1; | |
| } | |
| cooltime_units = cooltimeType; |
2 participants
This PR introduces changes that allow the user to select the units (default or seconds) in which cooling times are printed in the headers of the output tables, if the
unitskeyword is added in theoutputsection of the input file.Adding this in as a draft for now as this change has had many moving pieces, with changes to function definitions in multiple files, and I anticipate more work being necessary for everything to work as intended.
fixes #137