Repository navigation
Removing Legendre filter in diffusion coefficient results - #1909
Conversation
…ion coefficient results. With this fix, the Legendre filter is successfully sliced. Also, there was previously concern that get_condensed_xs needed to be specially catered toward DiffusionCoefficient. However, since the MGXS get_condensed_xs method condenses the tallies before calculating the cross section, that should work great for DiffusionCoefficient as well. Tests updated.
|
"The most important thing is that the diffusion coefficient not be condensed, and instead have the transport cross section condensed, and then the diffusion coefficient calculated based on that condensed transport cross section." are you sure about that? I could have sworn that despite it being counterintuitive, condensing 1/(3sigma_tr) preserves leakage better. |
|
Gavin is correct on this. Preserving 1/Sigma_tr preserves the flux weighted mean free path. Typically, lattice codes will spatially homogenized Sigma_tr and energy condense 1/Sigma_tr. This is only done to facilitate dealing with voids. Not super trivial to do in a single tally in MC, so you would need to post-process the energy condensation. That was the entire motivation behind doing the migration area work.
From: Gavin Ridley ***@***.***>
Sent: Tuesday, November 16, 2021 4:58 PM
To: openmc-dev/openmc ***@***.***>
Cc: Subscribed ***@***.***>
Subject: Re: [openmc-dev/openmc] Removing Legendre filter in diffusion coefficient results (PR #1909)
"The most important thing is that the diffusion coefficient not be condensed, and instead have the transport cross section condensed, and then the diffusion coefficient calculated based on that condensed transport cross section."
are you sure about that? I could have sworn that despite it being counterintuitive, condensing 1/(3sigma_tr) preserves leakage better.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub<#1909 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAHXBLWMPHRGDTR6NJDX22LUMLHVFANCNFSM5IFIVI5Q>.
Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
|
You're totally right (The rule is "don't condense transport cross sections" which I always mis-remember as diffusion coefficients!!). Anyway, here, it's all the tallies that are condensed. That includes total and scatter-1 which are then used in the transport cross section. Consequently, the transport correction is re-calculated every time you condense. This seems good to me since it's as if you tallied everything with the coarser group structure. |
|
@mkreher13 I took a quick look now and it looks good. I compared diffusion/nu-diffusion and the transport/nu-transport results from the tests and showed they are related as we expect. In all cases that I could test, including the condensation test, this was true. What I couldn't look at from here is the mgxs_library_nuclides test because that result is quite large and so our test reference is a hash. Could you (locally) convert that test to ASCII and check a sampling of values to ensure diffusion=1/(3*tr) and report back? |
|
Thanks @nelsonag! I ran the mgxs_library_nuclides test with hashing turned off. I compared the values, and I can confirm that diffusion is 1/(3*tr). |
|
Great @mkreher13, that sounds good to me. I've reviewed the code and it looks good to me as-is. One last thing though any idea why test_tally_aggregation is failing? |
|
It's the 8th decimal place on a single result that's off. This has been happening to me recently, sometimes it fails, sometimes it doesn't. I can't explain it. |
nelsonag
left a comment
nelsonag
left a comment
There was a problem hiding this comment.
Looks good to me, thanks @mkreher13
|
Since this PR should have no impact on that particular test behavior, I will go ahead and make the merge. Thanks again @mkreher13 |
Removing Legendre filter in diffusion coefficient results
4 participants
@nelsonag based on our conversation, here is my solution:
Fixed issue where both P0 and P1 moments were displayed in the diffusion coefficient results. With this fix, the Legendre filter is successfully sliced so all results display only the P1 results. My fix involves modeling the code more closely after the TransportXS class.
I deleted the method for
get_condensed_xsin theDiffuionCoefficientclass since it wasn't working. Somehow, I failed to notice that the results fromget_condensed_xsweren't condensed! Upon further investigation, I noted that the function just didn't work.Since the
MGXSget_condensed_xsmethod condenses the tallies before calculating the cross section, that is compatible with diffusion coefficient condensation. The most important thing is that the diffusion coefficient not be condensed, and instead have the transport cross section condensed, and then the diffusion coefficient calculated based on that condensed transport cross section. By condensing the tallies involved in the diffusion coefficient calculation, that is exactly what happens. Therefore, using the originalget_condensed_xsmethod in theMGXSclass instead makes sense and is accurate. I tested this on a simple example by making a mgxs_library that had 'transport' and 'diffusion-coefficient' specified. After running, I loaded the data and ensured that the condensed value of the diffusion coefficient was equal to 1/(3* condensed transport).I also tested my changes in my personal branch of transient code that leverages the diffusion-coefficient. After my changes, the results were identical and my transient tests passed.
All mgxs_library test results have been updated.