vyrjana / pyimpspec

A package for parsing, validating, analyzing, and simulating impedance spectra.
https://vyrjana.github.io/pyimpspec/
GNU General Public License v3.0
17 stars 5 forks source link

Lambda values ~>0.0001 DRT fail to calculate #5

Closed Aganosminormis closed 2 weeks ago

Aganosminormis commented 7 months ago

Specifications

Description: DRT calculations fail at certain lambda values producing the following error: new = pyimpspec.calculate_drt(data0, method='tr-rbf',lambda_value=lam) DRTError: Failed to perform calculations! Try tweaking the settings.

This occurs at a lambda value of ~0.0001.

This error only occurs with cvxpy (with the windows specific instructions followed)

With both cvxopt and kvxopt, erroneous DRT distributions are produced with negative values and extra oscillations. image

Expected behavior The system is a simple randle cell with a known characteristic time at 10^-1. pyDRTtools produces the correct results at various lambda values. image

vyrjana commented 7 months ago

Thanks for bringing this to my attention.

The implementation included in pyimpspec is based on commit 65ea54d9332a0c6594de852f0242a88e20ec4427 of pyDRTtools, but it looks like quite a few changes have been made to the pyDRTtools repository since then. At a quick glance it looks like they have switched over to using only cvxopt instead of being able to use either cvxpy or cvxopt depending on which is available (or if cvxpy raises an exception), and they have added the option to pick a suitable regularization parameter based on cross validation. I'm attaching two screenshots obtained with the old version of pyDRTtools (i.e., commit 65ea54d....) with a parallel RC circuit (R=10 kohm, C=10 µF) in the frequency range 0.1 Hz to 1 MHz and either λ=1e-2 (top) or λ=1e-4 (bottom).

1e-2

1e-4

The latest version of pyDRTtools (commit 3694b9b4cef9b29d623bef7300280810ec351d46), which might be the same version you used, is indeed able to produce better results (λ=1e-4 with the same impedance spectrum as described earlier):

new_1e-4

I will update the implementation included in pyimpspec.

vyrjana commented 7 months ago

It turns out that a non-negativity constraint was also added at some point. Here is an example of what I currently get when using my development branch after updating the implementation.

comparison

Aganosminormis commented 7 months ago

Thank you for your diligence regarding the issue. I would appreciate it if the pyimpspec implementation could be updated soon as having API access to DRT is incredibly convenient.

I would also appreciate it if pyimpspec could support other methods for picking the regularization parameter such as GCV, rGCV etc as is done in pyDRTtools.

vyrjana commented 6 months ago

The updated TR-RBF implementation requires some changes to the function signature, which is why it is not backwards compatible and the semantic versioning then requires that the version number is bumped up to 5.0.0. However, I'm already working on version 5.0.0, which introduces a lot of other changes that break backwards compatibility. That update is not yet ready for a public release and I would rather not bump the major version number frequently.

That said, if you are familiar with installing local Python wheels (or willing to figure out how to do it), then I could use Git's cherry-pick feature to put together a patched version of version 4.1.1 that applies the updated TR-RBF implementation and upload the wheel as an attachment to this issue thread. How does that sound? Keep in mind that I have not yet had time to do any extensive testing of the updated implementation.

And yes, support for most of the cross-validation methods is included in the updated implementation. The k-fold method is not included at the moment as I'm still considering whether or not to add yet another dependency (scikit-learn).

vyrjana commented 2 months ago

Version 5.0.0 has been released and should be available for download via PyPI.

Up2nothing commented 2 months ago

Great to see that pyimpspec has got a big upgrade! I have started using DearEIS and find is great so far. Would it be a lot of work in update DearEIS so that it can also benefit from the 5.0.0 upgrade that pyimpspec has revieved? Or should I open a feature request in the DearEIS project?

vyrjana commented 2 months ago

I'm finishing up work on DearEIS 5.0.0. The main parts that remain are updating the documentation and doing some testing to check for bugs.