Skip to content

Adding bifrustum model - #790

Open
sara-mokhtari wants to merge 4 commits into
py-Iqabcfrom
bifrustum_clean
Open

sara-mokhtari wants to merge 4 commits into
py-Iqabcfrom
bifrustum_clean

Conversation

@sara-mokhtari

@sara-mokhtari sara-mokhtari commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

Description

Fixes #772
Adds a pure-Python bifrustum model (truncated bipyramid with a regular polygonal cross-section).
Depends on #781 (support for pure Python 2D models): this PR is based on py-Iqabc and targets it,
so the diff only shows the bifrustum. Once #781 is merged, I will retarget this PR to master
and merge master into this branch.

  • Form factor computed by decomposing the shape into tetrahedra (Delaunay), with the singular cases
    handled by the closed-form expressions of Yang et al. (doi:10.1107/S160057672201130X)
  • 1D: orientation average with a Fibonacci quadrature (500 points)
  • 2D: Fqabc/Iqabc with theta, phi, psi

Note for review: computational optimizations were added (shared vertices: explained inside the code and documentation).

How Has This Been Tested?

  • model_test passes
  • F(q) checked against an independent method (Debye equation with Debye calculator: https://github.com/FrederikLizakJohansen/DebyeCalculator) for different truncation ratios. The Debye equation is based on atomic positions while SasView model is based on analytical expressions.
bifrustum_validation_debye
  • Tested on experimental data (gold truncated bifrustum: pentagonal cross-section, truncation ratio of ~ 0.2) were fitted as well for different sizes: fit values are similar to TEM values (figure below is an example of fit, the model was also tested on 3 other experimental data).
bifrustum_validation_exp_BP1600 Image

Review Checklist:

[if using the editor, use [x] in place of [ ] to check a box]

Documentation (check at least one)

  • There is nothing that needs documenting
  • Documentation changes are in this PR
  • There is an issue open for the documentation (link?)

Installers

  • There is a chance this will affect the installers, if so
    • Windows installer (GH artifact) has been tested (installed and worked)
    • MacOSX installer (GH artifact) has been tested (installed and worked)
    • Wheels installer (GH artifact) has been tested (installed and worked)

Licensing (untick if necessary)

  • The introduced changes comply with SasView license (BSD 3-Clause)

codescene-access[bot]

This comment was marked as outdated.

@sara-mokhtari
sara-mokhtari requested review from pkienzle and a balanced review from Copilot November 8, 2026 14:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

codescene-access[bot]

This comment was marked as outdated.

@codescene-access codescene-access Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No quality gates enabled for this code.

See analysis details in CodeScene

Quality Gate Profile: Custom Configuration
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

@pkienzle pkienzle 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.

Code looks okay. Models run.

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.

4 participants