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

generalizing code #199

Closed
SimonRubenDrauz opened this issue Jan 7, 2021 · 3 comments
Closed

generalizing code #199

SimonRubenDrauz opened this issue Jan 7, 2021 · 3 comments

Comments

@SimonRubenDrauz
Copy link
Collaborator

To generalize calculate derivatives and extract results on branch_models.py level might lead to problems in near future, as a most abstract class defines really real and concrete functions/classmethods. Therefore, there should be a discussion on how to make the code as abstract as possible, prevent duplicated code and seperate it from really concrete functions which should only be part of the real component models.

@dlohmeier dlohmeier mentioned this issue Jan 28, 2021
@dlohmeier
Copy link
Collaborator

dlohmeier commented Jan 6, 2022

This problem should be tackled with the numba development (cf. dev_numba branch or PR #335 or Issue #242 ). What I suggest there is:

  • make calculate_derivatives a global function to be called on the whole branch pit (actually, almost all components do nearly exactly the same thing, as you pointed out)
  • implement two class methods for all components for a priori and a posteriori adaption of the derivative values

Of course this means that we have to take care of a very general behavior for all types of branches, but we actually do that already and still go through all components individually instead of performing this step just once.

@dlohmeier
Copy link
Collaborator

The dev_numba branch is merged. The only feature we still miss somehow is a possibility to tell the global result extraction function which results to extract. Currently, this is somewhat hard coded and the components can decide whether they need these results and could extract further results for their own, but maybe there is a smarter way to do this.

@kbensafta kbensafta added question Further information is requested discussion? Discussion on further development required labels Feb 9, 2023
@dlohmeier
Copy link
Collaborator

The remaining problem already has an issue #374 .

@kbensafta kbensafta removed question Further information is requested discussion? Discussion on further development required labels Feb 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

No branches or pull requests

3 participants