Skip to content

Feature/mf26#143

Merged
whaeck merged 40 commits intodevelopfrom
feature/mf26
Mar 14, 2023
Merged

Feature/mf26#143
whaeck merged 40 commits intodevelopfrom
feature/mf26

Conversation

@whaeck
Copy link
Copy Markdown
Member

@whaeck whaeck commented Aug 18, 2021

And ... I'm done! Full MF26 capability ;-)

@whaeck whaeck requested a review from nathangibson14 August 18, 2021 02:02
@whaeck whaeck requested a review from ptalou February 8, 2023 23:04
Copy link
Copy Markdown
Contributor

@nathangibson14 nathangibson14 left a comment

Choose a reason for hiding this comment

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

Just a few pedantic comments.

*
* The ContinuumEnergyAngle class is used to represent the continuum
* energy-angle law=1 data of MF26. It is similar to the MF6 law=1, but only
* supports the LANG=1 option (Legendre coefficients).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pedantic comment... NA=0, so there are no coefficients 😄

*
* The ContinuumEnergyAngle class is used to represent the two-body angular
* distribution law=2 data of MF26. It is similar to the MF6 law=1, but only
* supports the LANG=1 option (Legendre coefficients).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This documentation is messed up. Looks like a copy and paste from ContinuumEnergyAngle.

std::move( boundaries ),
std::move( interpolants ),
std::move( energies ),
std::move( multiplicities ) ) {}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The ENDF manual says that the multiplicities must always be 1, not just "normally one". Thus, it doesn't really make sense to require it as an input. I think the only value of this constructor would be to make a user specify the energy range and the law.

That said, maybe consistency with MF6 is desired? So maybe this is okay?

return
" 1.100000+1 5.438673-4 0 2 2 2 10026525 \n"
" 2 2 10026525 \n"
" 1.000000+1 1.000000+0 1.00000+11 1.000000+0 10026525 \n";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we're going to leave multiplicity as a user input, we should have a checkMultiplicity function that makes sure it's all 1s.

@@ -0,0 +1,42 @@
/**
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing the Python default constructor.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The python bindings compile so it might not be required. I'll still add it to be safe.

std::string buffer;
auto output = std::back_inserter( buffer );
chunk.print( output, 9228, 6, 5 );
chunk.print( output, 9228, 26, 5 );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You changed all the 6s to 26s. I don't think you meant to do this.

@whaeck whaeck merged commit 6b79cc3 into develop Mar 14, 2023
@whaeck whaeck deleted the feature/mf26 branch March 14, 2023 17:17
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.

2 participants