Fix modified inductance element La to match its documented definition - #337
Open
shaurya20335 wants to merge 1 commit into
Open
shaurya20335 wants to merge 1 commit into
shaurya20335 wants to merge 1 commit into
Conversation
La computed (L * j * omega)^alpha, which also raises L to the power alpha. The docs (and EC-Lab's definition) give Z = L * (j * omega)^alpha. Fix the formula, update the La reference values in test_each_element, add a test that Z is linear in L, and correct the units of L to H sec^(a - 1). Fixes ECSHackWeek#312
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #312
The modified inductance element
Lais documented asbut the code computes
(L * 1j * omega) ** alpha, so L also gets raised to the power alpha. The two only agree when alpha = 1. For alpha < 1 the fitted L comes out wrong and its units don't make sense.Changes:
elements.py:Z = L * (1j * omega) ** alphaH sectoH sec^(a - 1), since L * omega^alpha has to be in Ohm. This follows the same style as the TLMQ element (F sec^(gamma - 1)).test_each_element: the old reference values forLamatched the buggy formula, so I recomputed them from L * omega^alpha * exp(j * alpha * pi / 2) with L = 0.1 and alpha = 0.2test_La_linear_in_L: doubling L should double Z for any alpha. It fails on the old code and passes with the fix.flake8is clean and the full test suite passes locally (36 passed).One thing to note for users: anyone who fitted circuits with
Labefore this will get a different (now correct) L value if they refit. The alpha values and the fit quality are not affected, since the old form is just a reparameterisation of L.