Skip to content

Drop cubic fit for interpolated broadening#473

Draft
ajjackson wants to merge 20 commits intorelease-2.0.0from
drop-cubic-fit
Draft

Drop cubic fit for interpolated broadening#473
ajjackson wants to merge 20 commits intorelease-2.0.0from
drop-cubic-fit

Conversation

@ajjackson
Copy link
Copy Markdown
Member

@ajjackson ajjackson commented Apr 7, 2026

Close #282

  • Remove arguments selecting between cubic and cheby-log fits
    • Despite meaning the same thing these have various names: width_fit, adaptive_fit, adaptive_error_fit
    • Initially removed from other modules, but can also remove from broadening module
  • Observe that all unit test failures are (small) value differences in broadened spectra
  • Update test data

ajjackson and others added 16 commits February 13, 2026 10:04
Following established conventions for python list and numpy array, if
a slice exceeds the array length the result can quietly return up to
the last available item. This allows things like [:10] to mean "up to
first ten items" which is nice for printing etc.
This makes the API more resilient against changes (e.g. because a new
algorithm takes extra arguments). It is a breaking change so should be
part of Euphonic 2.0 release.
Co-authored-by: Jacob Wilkins <46597752+oerc0122@users.noreply.github.com>
some very funny business going on with github suggest-changes indentation, let's see if this works
"Suggested change" feature seems to be somewhat broken now?
* Use dotnet tools to create/upload coverage report as build artifact

* Adopt orgoro coverage action as used by castep_outputs

* Upload SVG coverage badge artifact and commit to "badges" branch when appropriate
  - if running on `master`
  - or if specifically requested as workflow dispatch option

* Add badge to README
* Restructure error messages to standard format

* Implement some utilities to help use this

* Enable write permission for orgoro coverage

---------

Co-authored-by: Adam J. Jackson <a.j.jackson@physics.org>
Currently, calls with a positional argument give inconsistent results
between the two classes. Adding a default also improves
compatibility/consistency in API.
* Fix changelog check

---------

Co-authored-by: Adam J. Jackson <adam.jackson@stfc.ac.uk>
The parser argument still exists because it is added from the "brille"
feature group then hidden in this specific tool. Removing items from a
parser seems to require private methods and is discouraged.

So, instead we just drop that parameter from the parsed parameters
before passing them to check_brille_settings
@ajjackson ajjackson changed the title Drop cubic fit Drop cubic fit for interpolated broadening Apr 7, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2026

Test Results

   22 files   -     25     22 suites   - 25   18m 9s ⏱️ - 30m 59s
1 115 tests  -      2  1 101 ✅  -      9   6 💤 ± 0   8 ❌ + 8 
9 948 runs   - 16 612  9 819 ✅  - 16 593  57 💤  - 90  72 ❌ +72 

For more details on these failures, see this check.

Results for commit c7e604a. ± Comparison against base commit 6132c68.

♻️ This comment has been updated with latest results.

We only really have/use cheby-log so not much point in making it a
parameter. There is room for improvement, but this might not be the
right API when it comes, anyway.
@ajjackson
Copy link
Copy Markdown
Member Author

The summaries of differences in test data look reasonable. I'd like to do a bit of plotting/benchmarking first to be sure they aren't changing in the "wrong" direction away from non-approximated values.

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