Skip to content

Enable specifying reference direction for azimuthal angle in PolarAzimuthal distribution - #3582

Merged
GuySten merged 13 commits into
openmc-dev:developfrom
GuySten:polar-fix
Nov 3, 2025
Merged

GuySten merged 13 commits into
openmc-dev:developfrom
GuySten:polar-fix

Conversation

@GuySten

@GuySten GuySten commented Sep 25, 2025 •

Copy link
Copy Markdown
Contributor

Description

Currently the reference direction for the azimutal angle in PolarAzimuthal distribution is not clear.

This PR enable specifying the direction in the python API so it is clear what is going on and what is the direction that corresponds to a specific phi

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 15) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@GuySten
GuySten marked this pull request as ready for review September 25, 2025 11:09
@GuySten GuySten changed the title Enable specifying reference direction for azimutal angle in PolarAzimuthal distribution Enable specifying reference direction for azimuthal angle in PolarAzimuthal distribution Sep 27, 2025
@shimwell

shimwell commented Oct 31, 2025 •

Copy link
Copy Markdown
Member

I noticed the point detector work would benefit from this PR and wondered if it is worth putting in a couple of tests. I can PR to your branch if that helps

def test_reference_vwu_projection():
    """When a non-orthogonal vector is provided, the setter should project out
    any component along reference_uvw so the stored vector is orthogonal.
    """
    pa = stats.PolarAzimuthal()  # default reference_uvw == (0, 0, 1)

    # Provide a vector that is not orthogonal to (0,0,1)
    pa.reference_vwu = (2.0, 0.5, 0.3)

    reference_v = np.asarray(pa.reference_vwu)
    reference_u = np.asarray(pa.reference_uvw)

    # reference_v should be orthogonal to reference_u
    assert abs(np.dot(reference_v, reference_u)) < 1e-6


def test_reference_vwu_normalization():
    """When a non-normalized vector is provided, the setter should normalize
    the projected vector to unit length.
    """
    pa = stats.PolarAzimuthal()  # default reference_uvw == (0, 0, 1)

    # Provide a vector that is neither orthogonal to (0,0,1) nor unit-length
    pa.reference_vwu = (2.0, 0.5, 0.3)

    reference_v = np.asarray(pa.reference_vwu)

    # reference_v should be unit length
    assert np.isclose(np.linalg.norm(reference_v), 1.0, atol=1e-12)

@GuySten

GuySten commented Oct 31, 2025

Copy link
Copy Markdown
Contributor Author

Thanks @shimwell. I invite you to push the tests to my PR.

@shimwell

Copy link
Copy Markdown
Member

Thanks @shimwell. I invite you to push the tests to my PR.

PR made GuySten#33

@GuySten

GuySten commented Nov 3, 2025

Copy link
Copy Markdown
Contributor Author

@shimwell, are you interested in reviewing this PR as well?

@shimwell

shimwell commented Nov 3, 2025

Copy link
Copy Markdown
Member

Yes sorry I did take a look at everything and those tests (thanks for fixing) were the only thing I thought it was missing. So this looks good to me. LGTM

@shimwell shimwell self-assigned this Nov 3, 2025
@shimwell
shimwell self-requested a review November 3, 2025 06:15
@shimwell shimwell removed their assignment Nov 3, 2025
@GuySten
GuySten merged commit fd964bc into openmc-dev:develop Nov 3, 2025
15 checks passed
@GuySten
GuySten deleted the polar-fix branch November 3, 2025 07:43

@paulromano paulromano left a comment

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.

Sorry to chime in too late here -- the name reference_vwu doesn't really make sense to me. I'll submit a follow-on PR to suggest a new name.

apingegno pushed a commit to apingegno/openmc that referenced this pull request May 7, 2026
…muthal distribution (openmc-dev#3582)

Co-authored-by: shimwell <mail@jshimwell.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants