Skip to content

Add uncertainties for main parameters. - #299

Merged
sbillinge merged 7 commits into
diffpy:mainfrom
Sparks29032:covariance
Jun 1, 2026
Merged

Add uncertainties for main parameters.#299
sbillinge merged 7 commits into
diffpy:mainfrom
Sparks29032:covariance

Conversation

@Sparks29032

@Sparks29032Sparks29032 commented Apr 24, 2026•
edited
Loading

Copy link
Copy Markdown
Collaborator

Closes#300

Need tests for warnings and each parameter separately (including baselineslope).

Need special handling for squeeze and func[x/y/xy] morph parameters.

image

@codecov

codecovBot commented Apr 24, 2026•
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.93%. Comparing base (fc45fa1) to head (d64d8d8).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #299 +/- ##
=======================================
Coverage 99.93% 99.93% =======================================
Files 24 24 Lines 1480 1523 +43 =======================================
+ Hits 1479 1522 +43 
Misses 1 1
Files with missing linesCoverage Δ
tests/test_morphapp.py100.00% <ø> (ø)
tests/test_morphio.py100.00% <100.00%> (ø)
tests/test_morphpy.py100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Sparks29032Sparks29032 mentioned this pull request Apr 24, 2026
Closed
@Sparks29032

Sparks29032 commented May 11, 2026•
edited
Loading

Copy link
Copy Markdown
CollaboratorAuthor

Case where all parameters have uncertainties:
image

Case where certain parameters cause the Hessian to turn singular:
image

Tests and documentation updated. Ready to merge after review.

@Sparks29032

Sparks29032 commented May 11, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Oops, previous one I ran only one of the tests I thought would be affected... fixed now.

@Sparks29032

Sparks29032 commented May 11, 2026

Copy link
Copy Markdown
CollaboratorAuthor
image

Loosened tolerance for checking numerics match. New algorithms and updates to old ones can cause some deviation (e.g. works on one of our tests on PR workflows but not others).

@Sparks29032
Sparks29032 marked this pull request as ready for review May 11, 2026 06:17
sbillinge
sbillinge reviewed May 15, 2026
View reviewed changes

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

looks great. One inline comment.

Comment threadsrc/diffpy/morph/morph_io.py Outdated
f"# {param} = {morph_results[target][param]:.6f}"
for param in morph_results[target]
)
# output += "# Optimized morphing parameters:\n"

@sbillingesbillingeMay 15, 2026

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.

are you retaining these lines for some reason? If yes, state the reason and when they can be removed, and if not, then just remove them.

@Sparks29032Sparks29032May 15, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Ah, thanks. I left that there as reference for a different approach originally.

@Sparks29032

Sparks29032 commented May 15, 2026

Copy link
Copy Markdown
CollaboratorAuthor

@sbillinge Removed the stray comment.

Since pre-commit isn't working on here (we will address by #301 hopefully), this screenshot is just to confirm it is passing it locally.
image

@sbillinge

sbillinge commented May 19, 2026

Copy link
Copy Markdown
Contributor

@Sparks29032 Please could you fix pre-commit. Check scikit-package for the update. We need to update versions on pretty much everything, but there are specific changes to docformatter that should fix this bug and get tests passing.

@sbillinge

sbillinge commented May 31, 2026

Copy link
Copy Markdown
Contributor

this still seems to be failing tests @Sparks29032 . I didn't look why.

@Sparks29032

Sparks29032 commented Jun 1, 2026

Copy link
Copy Markdown
CollaboratorAuthor

Force-push to remove rebase. Something went wrong there.

@Sparks29032

Sparks29032 commented Jun 1, 2026

Copy link
Copy Markdown
CollaboratorAuthor

No more conflicts now. Good to merge.

@sbillinge
sbillinge merged commit cb901e8 into diffpy:mainJun 1, 2026
7 checks passed
Sparks29032 added a commit to Sparks29032/diffpy.morph that referenced this pull request Jun 1, 2026
@Sparks29032@pre-commit-ci
Add uncertainties for main parameters. (diffpy#299)
d1129d7
* Add uncertainties for main parameters.
* Works for all parameters now
* Fix other tests
* loosen tolerance to 1e-4 from 1e-5
* Lower tolerance more
* Remove comment
* [pre-commit.ci] auto fixes from pre-commit hooks
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
sbillinge pushed a commit that referenced this pull request Jun 1, 2026
@Sparks29032@pre-commit-ci
Give user option to save on original grid (#304)
2c7dbe5
* Allow original grid saving
* Add uncertainties for main parameters. (#299)
* Add uncertainties for main parameters.
* Works for all parameters now
* Fix other tests
* loosen tolerance to 1e-4 from 1e-5
* Lower tolerance more
* Remove comment
* [pre-commit.ci] auto fixes from pre-commit hooks
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* Rebasing
* Remove duplicated test
* Indent
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sbillingesbillingesbillinge left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add option to see uncertainties

2 participants

@Sparks29032@sbillinge