eitcom / pyEIT

Python based toolkit for Electrical Impedance Tomography
Other
169 stars 96 forks source link

Issue setup small fix + optional meas_pattern #45

Closed DavidMetzIMT closed 2 years ago

DavidMetzIMT commented 2 years ago

Hy @liubenyuan,

I saw you merged my precedent PR. could you already test it? However I saw a small issue by trying to not pass ex_mat or perm to following method! solve, solve_eit, compute_jac and compute_b_matrix.

Moreover I have implemented an option to pass throught **kwargs a custom meas_patttern to solve_eit, compute_jac and compute_b_matrix and to EitBase!

liubenyuan commented 2 years ago

The voltage_meter might fail for eit_stack_jac.py. I think voltage_meter should be part of a measurement protocol and this function should favor clarity rather than speed. The diff_op can be calculate and initialized once every EITForward class by passing ex_mat, step and parser (as these parameters are all protocol related for a EIT system).

I am rewriting the interface for EITForward class and these apis may subject to changes. We can discuss on this.

More detail:

Forward(mesh, el_pos, ref_el) -> class
  -> solve
  -> EITForward(ex_mat, step, parser) -> class
      -> solve_eit
      -> jac, b
DavidMetzIMT commented 2 years ago

Hy,

The voltage_meter might fail for eit_stack_jac.py. I think voltage_meter should be part of a measurement protocol and this function should favor clarity rather than speed. The diff_op can be calculate and initialized once every EITForward class by passing ex_mat, step and parser (as these parameters are all protocol related for a EIT system).

you mean eit_static_jac.py? I couldn't see any pb can you say more about that?

I am rewriting the interface for EITForward class and these apis may subject to changes. We can discuss on this.

More detail:

Forward(mesh, el_pos, ref_el) -> class
  -> solve
  -> EITForward(ex_mat, step, parser) -> class
      -> solve_eit
      -> jac, b

you want to make a base class Forward and make and child class EitForward? solve need also ex_mat, and the call of EITForward would be:

EITForward(mesh, el_pos, ref_el, ex_mat, step, parser)

by the way mesh, el_pos, ref_el shoul be part of a mesh class (I started this but I wanted first you to get this fixs)... and ex_mat, step, parser could be part of a stimulation class like in EIDORS! or actually it would be better to pass ex_mat and meas_pattern instead ex_mat, step, parser and do like for ex_mat an external function to compute meas_pattern

Something like that:

by passing objs to to forward you avoid increasing the nb of arg and by need you can add a parameter simply by adding it in the parameter class! also these classes can o much more on the data they contain (check them, have additional prop relative to the data they contain, see PyEITMesh)


@dataclass
class PyEITMesh
     node
     element
     perm
     el_pos
     ref_el

    @proprety
    def n_nodes:
    @proprety
    def n_el:

ex_mat= eit_scan_lines(n_el, step)

meas_mat= voltage_meter(ex_mat, step, parser)

@dataclass
class PyEITStim
     ex_mat
     meas_mat

class Forward(mesh: PyEITMesh, stim: PyEITStim)
    mesh
    stim
    def solve:
    def solve_eit:
    def jac:
    def b:

for these PR I would invite you to add the small fix and the additional option to add a custom meas_pattern is very usefull for special electrode array!

did you already made some changes? let me know about that! can you push it on a test branch? so that we can work together on that?

liubenyuan commented 2 years ago

sure, I have modify these change based on your previous commit. I will merge your this commit and push to a dev branch so we can work together.

solve (Forward class) only need to know one stimulation pattern (ex_line). And only in this way it would be much easier to port to the complete electrode model.

solve_eit, compute_jac and smear (EITForward class) needs to know the complete protocol (stimulation patterns, ex_mat), step and parser. ex_mat specifies the stimulation, while step and parser forms the measurement pattern. Currently we do not need (or do we?) to combine step and parser into meas_pattern, as it would need a wrapper function to generate a meas_pattern for a EIT system.

DavidMetzIMT commented 2 years ago

solve_eit, compute_jac and smear (EITForward class) needs to know the complete protocol (stimulation patterns, ex_mat), step and parser. ex_mat specifies the stimulation, while step and parser forms the measurement pattern. Currently we do not need (or do we?) to combine step and parser into meas_pattern, as it would need a wrapper function to generate a meas_pattern for a EIT system.

I think it is a good idee only to pass ex_mat and meas_pattern and the wrapper function is voltage_meter! So you give the possibility to the user to use your meas_pattern or a custom one! actually like ex_mat!

DavidMetzIMT commented 2 years ago

Ok send me a message when have created the dev branch

liubenyuan commented 2 years ago

@DavidMetzIMT The latest changeset are commit to the dev branch. And please also see issue #44, #46