Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add jac_normalized in setup parameter for jac and remove init flag from solve_eit,... #51

Merged
merged 6 commits into from
May 10, 2022

Conversation

DavidMetzIMT
Copy link
Contributor

Hy @liubenyuan,

I saw that we forget the jac_normalized option for jac. is it also needed for greit? If yes I can added it!

By the way I just forget to ask you why you introduced the init flag on solve_eit, compute_b_matrix, compute_jac_matrix.
I think the init flag can be removed! each time you set it to true you pass a perm vector and viceversa, so it make it the flag redondant.
I would suggest we recalculate kg only when perm is not None. It is maybe more intuitive, I was confused by using it in my own sofware. I made made some modifications (which made fem even more lighter!)

@liubenyuan
Copy link
Collaborator

liubenyuan commented May 10, 2022

Yes, this parameter jac_normalized should be kept for the analysis of the sensitivity distributions.

I added a note on using perm = None or user_perm != perm to trigger a recalculation of se and kg. I think your modification dropping init=True is truly intuitive to use!

The warning message on passing perm=None can be suppressed, as it is the desired behavior (default) of calculating se from mesh.perm. We can add a note on this parameter rather than throw a warning message.

@DavidMetzIMT
Copy link
Contributor Author

Ok I deleted the warning and added also jac_normalized to the setup parameters in greit solver!

@liubenyuan liubenyuan merged commit 4324660 into eitcom:master May 10, 2022
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.

3 participants