AUTODIAL / AutoEIS

A tool for automated extraction of equivalent circuit models (ECM) from electrochemical impedance spectroscopy (EIS) data
https://autodial.github.io/AutoEIS/
MIT License
33 stars 7 forks source link

`perform_full_analysis` called with wrong argument order in documentation #93

Closed benjamin5988 closed 8 months ago

benjamin5988 commented 9 months ago
import numpy as np
import autoeis as ae

# Load test dataset shipped with AutoEIS
Z, freq = ae.io.load_test_dataset()

# Perform automated EIS analysis
circuits = ae.perform_full_analysis(Z, freq, iters=100, parallel=False)
print(circuits)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[4], line 5
      2 Z, freq = ae.io.load_test_dataset()
      4 # Perform automated EIS analysis
----> 5 circuits = ae.perform_full_analysis(Z, freq, iters=100, parallel=False)
      6 print(circuits)

File [~/miniforge3/envs/basic_3.11/lib/python3.11/site-packages/autoeis/core.py:808](http://localhost:8888/lab/tree/~/miniforge3/envs/basic_3.11/lib/python3.11/site-packages/autoeis/core.py#line=807), in perform_full_analysis(freq, Z, iters, parallel, linKK_threshold, tol, num_warmup, num_samples)
    780 """Performs automated EIS analysis by generating plausible ECMs that
    781 fit the impedance data, followed by Bayesian inference on components.
    782 
   (...)
    805     Dataframe containing circuits, parameters, and MCMC results.
    806 """
    807 # Filter out bad impedance data
--> 808 Z, freq, rmse = preprocess_impedance_data(Z, freq, threshold=linKK_threshold)
    810 # Generate a pool of potential ECMs via an evolutionary algorithm
    811 kwargs = {"iters": iters, "complexity": 12, "tol": tol, "parallel": parallel}

File [~/miniforge3/envs/basic_3.11/lib/python3.11/site-packages/autoeis/core.py:160](http://localhost:8888/lab/tree/~/miniforge3/envs/basic_3.11/lib/python3.11/site-packages/autoeis/core.py#line=159), in preprocess_impedance_data(impedance, freq, threshold, plot)
    158 mask_phase = [True] * len(Im_Z)
    159 for i in range(len(Im_Z)):
--> 160     if i < index:
    161         mask_phase[i] = False
    163 freq = freq[index[0] :]

ValueError: The truth value of an array with more than one element is ambiguous. Use a.any() or a.all()
ma-sadeghi commented 9 months ago

Thanks for the bug report. For now, please use the step-by-step method (see the example notebook) until I figure out what's wrong with the envelope function.

There's a lot going on in between so there are many ways the envelope function can go wrong, and apparently we haven't covered all our bases

ma-sadeghi commented 9 months ago

@benjamin5988 Upon inspecting the error traceback, it seems that your code didn't get very far. It got stuck in the preprocessing step, for that I would need a minimal working example to debug. Basically, a script that "only" reproduces the bug, no more. Please include the data as well as I suspect it's the data (not that your data is the issue, but your data has revealed the potential bug, so I'll need it to reproduce and provide a fix)

benjamin5988 commented 9 months ago

The data is the test data. I’m just doing step by step. It’s fine - no worries.

On Thu, Feb 15, 2024 at 1:52 PM Amin Sadeghi @.***> wrote:

@benjamin5988 https://github.com/benjamin5988 Upon inspecting the error traceback, it seems that your code didn't get very far. It got stuck in the preprocessing step, for that I would need a minimal working example to debug. Basically, a script that "only" reproduces the bug, no more. Please include the data as well as I suspect it's the data (not that your data is the issue, but your data has revealed the potential bug, so I'll need it to reproduce and provide a fix)

— Reply to this email directly, view it on GitHub https://github.com/AUTODIAL/AutoEIS/issues/93#issuecomment-1947163676, or unsubscribe https://github.com/notifications/unsubscribe-auth/AYCJLMHUJKM5467232WFNMLYTZRRDAVCNFSM6AAAAABDIX6VCSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBXGE3DGNRXGY . You are receiving this because you were mentioned.Message ID: @.***>

megrez-light commented 9 months ago

I ran into the same issue, but that's actually the documentation mixing the order of the input parameters, here should be the right way to run the sample dataset (note the switch between the freq and Z argument:

import numpy as np
import autoeis as ae

# Load test dataset shipped with AutoEIS

Z, freq = ae.io.load_test_dataset()

# Perform automated EIS analysis

circuits = ae.perform_full_analysis(freq, Z, iters=100, parallel=False)
print(circuits)

It would be great if the documentation could be updated to reflect the actual order of the arguments.

ma-sadeghi commented 8 months ago

@megrez-light, @benjamin5988: Thanks again for the bug report, the documentation is now fixed.