Skip to content

refactor: change get_objective_function into a static method and getter#174

Merged
sbillinge merged 6 commits intodiffpy:john-developmentfrom
john-halloran:declass-obj
Nov 26, 2025
Merged

refactor: change get_objective_function into a static method and getter#174
sbillinge merged 6 commits intodiffpy:john-developmentfrom
john-halloran:declass-obj

Conversation

@john-halloran
Copy link
Contributor

  • get_objective_function() now passes to compute_objective_function()
  • wrote unit tests for compute_objective_function()

@sbillinge
Copy link
Contributor

do we know why tests are not running in CI?

@john-halloran
Copy link
Contributor Author

do we know why tests are not running in CI?

Out of credits, maybe? Otherwise I have no idea.

@john-halloran
Copy link
Contributor Author

It is saying:
[Invalid workflow file: .github/workflows/tests-on-pr.yml#L12](https://github.com/diffpy/diffpy.snmf/actions/runs/19680434710/workflow) error parsing called workflow ".github/workflows/tests-on-pr.yml" -> "Billingegroup/release-scripts/.github/workflows/_tests-on-pr.yml@v0" : workflow was not found. See https://docs.github.com/actions/learn-github-actions/reusing-workflows#access-to-reusable-workflows for more information.

@sbillinge
Copy link
Contributor

Ok, can we fix that on this pr?

@john-halloran
Copy link
Contributor Author

Ok, can we fix that on this pr?

Yes, it's fixed now. It broke because the scripts were moved to scikit-package from release-scripts and it didn't know to look for them there. Worth noting if this happens elsewhere.

@john-halloran
Copy link
Contributor Author

These issues cropping up are making me realize the package is a bit "dirty", probably because it originated from an old fork. I can keep patching things but a better solution might be to re-cut the package with scikit-package.

@codecov
Copy link

codecov bot commented Nov 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (john-development@d83619e). Learn more about missing BASE report.

Additional details and impacted files
@@                 Coverage Diff                 @@
##             john-development     #174   +/-   ##
===================================================
  Coverage                    ?   77.50%           
===================================================
  Files                       ?        3           
  Lines                       ?       40           
  Branches                    ?        0           
===================================================
  Hits                        ?       31           
  Misses                      ?        9           
  Partials                    ?        0           
Files with missing lines Coverage Δ
tests/test_snmf_optimizer.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@john-halloran
Copy link
Contributor Author

It passed now. The codecov score is cheating a bit, because one of the tests spins up the whole class. Ideally we'd get all our coverage through unit tests, and this PR is meant to be a step in that direction.

@sbillinge
Copy link
Contributor

I will merge this. But I agree, let's do a quick recut so we are sure everything is up to date. Let's also migrate to 3.12 - 3.14 while we are at it

@sbillinge sbillinge merged commit 9b0c509 into diffpy:john-development Nov 26, 2025
6 checks passed
@john-halloran john-halloran deleted the declass-obj branch December 1, 2025 20:37
sbillinge added a commit that referenced this pull request Feb 19, 2026
* Initial commit of John refactor of SNMF (#140)

* added my files

* [pre-commit.ci] auto fixes from pre-commit hooks

* Updated X and Y optimizers, now produces formatted but partially incorrect output

* removed duplicate class

* Working build (no normalization)

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* fix stop condition and add Y/A normalization (#141)

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* Added x normalization (#143)

Co-authored-by: John Halloran <jhalloran@oxy.edu>
Co-authored-by: Simon Billinge <sbillinge@users.noreply.github.com>

* Loop cleanup and scikit-learn style parameters (#146)

* Cleaned up looping behavior and aligned some params with scikit-learn.

* Combined quadratic solvers

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* feat: Add random state feature. (#150)

* feat: Add random state feature.

* Add class docstring

* components->n_components

* Updated docstring

* Shorten and reformat docstring

* docstring typo

* Flag self.rng as private

* Make logic for n_components and Y0 more rigid

* added class attributes to docstring

* fix: cleaner import of SNMFOptimizer

* fix: correct class instantiation after change in import

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>
Co-authored-by: Simon Billinge <sbillinge@users.noreply.github.com>

* refactor: Remove old tests and source files (#160)

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* style: convert variables to lowercase (#159)

* style: Convert variables to lowercase

* chore: Add news item

* style: lowercase remaining variables

* style: fix indentation on docstrings

* style: rename and make public certain attributes

* style: update docstring defaults

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* Fixes to class attributes and style (#163)

* fix: use symmetric initial phase fractions

* style: don't store objective function in a class attribute, just use history

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* Restored functionality to working state (#165)

* docs: update docstring and imports

* style: remove non-class function from class

* fix: using working quadratic solver for weights

* fix: restore working MATLAB-like loop functionality

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* feat: add live plotting of updates (#166)

* feat: add live plotting of updates

* style: make plotting vars lowercase

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* test: add initial test of optimizer (#167)

* test: add initial test of optimizer

* style: switch to local imports and plural folders

* style: add README to sNMF test

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* Replace 1D apply_interpolation with np.interp (#168)

* refactor: get residual matrix without a helper

* perf: remove unused derivatives from apply_interpolation

* chore: remove old residual matrix and reference to derivatives

* refactor: replace remaining apply_interpolation with np.interp

* style: remove references to old variable names

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* refactor: reconstruct separate from residuals (#169)

* refactor: reconstruct separate from residuals

* fix: inputs to reconstruct_matrix should not be optional

* docs: updated default documentation for init_weights

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* refactor: move optimization out of init and into fit (#170)

* style: per sklearn, attributes learned from data end with underscore

* refactor: move optimization out of init and into fit

* fix: use new fit() for test

* style: end fit() with return self

* docs: update docstring to reflect new functionality

* refactor: use rho and eta in fit

* feat: use reset for sequential refinements

* fix: use copies for safety

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* refactor: apply_interpolation_matrix() is now compute_stretched_components() (#171)

* fix: guard against zero/NaN stretches in apply_interpolation_matrix

* refactor: use broadcasting instead of np.tile in apply_interpolation_matrix

* refactor: flatten from a single buffer in apply_interpolation_matrix()

* refactor: drastically simplify indexing in apply_interpolation_matrix() and remove legacy MATLAB terminology

* style: rename apply_interpolation_matrix() to compute_stretched_components()

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* refactor: change get_objective_function into a static method and getter (#174)

* refactor: compute objective function in a static method and retrieve via getter

* refactor: change get_objective_function into a static method and getter

* fix: update location for _tests-on-pr.yml

* fix: use expected name for requirements/tests.txt

* fix: add cvxpy to requirements/conda.txt

* fix: add matplotlib to requirements/conda.txt

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* skpkg: apply black to all files in the project directory (#176)

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* chore: update package to latest scikit-package version (#178)

* chore: update to latest version of scikit-package

* chore: restore citation information

* fix: restore missing requirements

* [pre-commit.ci] auto fixes from pre-commit hooks

* fix: restore historical changelog

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* feat: skip updating stretch if stretching factor is zero (#181)

* style: make line length <80 throughout

* feat: skip updating stretch() if stretching factor is zero

* chore: fix codespell complaint

* fix: use correct version of rho

* style: clean up several lines and remove noqa

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* docs: revamp documentation (#182)

* refactor: move doc to docs and update requirements

* docs: remove old tables and recreate getting started page

* docs: load documentation from snmf_class.py

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>

* refactor: change references of snmf to stretched-nmf (#185)

* chore: use new name in scikit-package space

* refactor: rename references throughout program snmf -> stretched-nmf

* refactor: rename project references

* chore: add news item for package-name-update

* chore: CI toml issue

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>

---------

Co-authored-by: John Halloran <jhalloran@oxy.edu>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Simon Billinge <sbillinge@users.noreply.github.com>
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