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

koren model fixes #147

Merged
merged 4 commits into from
May 14, 2023
Merged

koren model fixes #147

merged 4 commits into from
May 14, 2023

Conversation

Federerer
Copy link
Collaborator

@Federerer Federerer commented Mar 30, 2022

I've changed grid current calculation to method proposed by Cohen and Helie in [1], [2] that we currently use in Pentodes. Polynomial smoothing helps with stability. I've also noticed, that Koren model gives better fitness e.g:
image
vs
image
(images are from this tool)
I've added additional parameters to the model to represent the new model variables. Unfortunately, getting correct values for all vacuum types will be difficult, as grid current vs grid voltage plots are not common in old datasheets 😢

@Federerer Federerer marked this pull request as ready for review May 14, 2023 00:18
@Federerer Federerer force-pushed the feature/koren_model branch from f605a37 to 7f675ad Compare May 14, 2023 00:20
@Federerer Federerer requested a review from dsharlet May 14, 2023 00:20
Copy link
Owner

@dsharlet dsharlet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If one of these models is clearly better than the others, we should consider just dropping the rest entirely. It would be nice not to need to expose such details. I guess we still need Koren for pentodes though?

@Federerer
Copy link
Collaborator Author

If one of these models is clearly better than the others, we should consider just dropping the rest entirely. It would be nice not to need to expose such details. I guess we still need Koren for pentodes though?

It's not that simple, Koren model is slightly faster and is more precise in terms of model accuracy, but it's also less stable in high gain situations. I think that setting a sensible default (Dempwolf-Zolzer) and leaving the option to change that for more advanced users is nice. BTW. Pentodes are now using Koren model with Cohen-Helie grid current calculation, which helps with the stability. 😉

@Federerer
Copy link
Collaborator Author

Federerer commented May 14, 2023

When testing this I've noticed how much changes I made to the solve/compile pipeline that we are using. On master, ReaRoute ASIO fails when you change simulation settings or add a probe, because of excessive locking on the real-time thread. I think I have to split my work somehow into a separate PR's because the dynamic parameter work takes longer than expected and there is a lot of stuff in the VST plugin that has to be implemented.

Circuit from #193 work fine on this branch with extreme amounts of gain.

@dsharlet
Copy link
Owner

Looks good, thanks!

When testing this I've noticed how much changes I made to the solve/compile pipeline that we are using. On master, ReaRoute ASIO fails when you change simulation settings or add a probe, because of excessive locking on the real-time thread. I think I have to split my work somehow into a separate PR's because the dynamic parameter work takes longer than expected and there is a lot of stuff in the VST plugin that has to be implemented.

I'd definitely prefer to see smaller PRs that can be merged independently. I know that with dynamic parameters though, there's going to be one big PR at some point for that, even if you can peel off some smaller stuff first.

@dsharlet dsharlet merged commit 2714291 into dsharlet:master May 14, 2023
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.

2 participants