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

BundleSolver setup too restrictive #192

Closed
shuheng-liu opened this issue Jan 7, 2023 · 1 comment · Fixed by #193
Closed

BundleSolver setup too restrictive #192

shuheng-liu opened this issue Jan 7, 2023 · 1 comment · Fixed by #193
Assignees
Labels
bug Something isn't working code quality enhancement New feature or request

Comments

@shuheng-liu
Copy link
Member

The following code block assumes each bundle parameter must fall into one of the following categories.

  1. a parameter that only appears in the boundary/initial condition (NOT in the equation); or
  2. a parameter that only appears in the equation (NOT in the boundary/initial condition)
    non_var = []
    for c in conditions:
    try:
    bc = tuple(c.bundle_conditions.values())
    except Exception:
    bc = ()
    for index in bc:
    non_var.append(index)
    non_var = list(set(non_var))
    def non_var_filter(*variables):
    variables = list(variables)
    for i, n in enumerate(non_var):
    del variables[len(conditions) + 1 + n - i]
    return ode_system(*variables)

However, in theory, there should be a case where the parameter appears both in the equation and in the initial/boundary condition. The var here probably implies a parameter that appears only in the equation. A fix is needed.

@shuheng-liu shuheng-liu added the enhancement New feature or request label Jan 7, 2023
@shuheng-liu shuheng-liu self-assigned this Jan 7, 2023
@shuheng-liu
Copy link
Member Author

shuheng-liu commented Jan 7, 2023

Plus, here's my interpretation of line 1331. It might be helpful to anyone else trying to understand what the non_var_filter is doing.

  • len(conditions) is the number of functions to solve for
  • + 1 is hard-coded number of free variables/coordinates/inputs to functions. Since this is an ODE solver, there's only 1 input.
  • + n is the index of a var (a parameter that appears only in the condition). It's deleted since the var doesn't appear in the equation by assumption.
  • - i accounts for the fact that at step i (starting from 0), a total of i vars have been previously deleted. This assumes previously deleted vars appear before the current var (i.e., non_var list is sorted)! Which is not guaranteed by line 1326 and can lead to unnoticeable bugs!

@shuheng-liu shuheng-liu added bug Something isn't working code quality labels Jan 7, 2023
@shuheng-liu shuheng-liu mentioned this issue Jan 8, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working code quality enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant