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

changed imports and the way the problem parameters are handled #48

Closed
wants to merge 10 commits into from

Conversation

flabowski
Copy link

Before merging, check #47

Changes:

  • Autoformat for conformance to the PEP 8 convention
  • replaced many from xy import * statements
  • introduced NS_namespace holding possible function arguments in the main executables
  • replaced vars().update(some_dictionary) with NS_namespace.update(some_dictionary)
  • All default parameters for all solvers are now only defined in NSfrac.__init__.py or NScoupled.__init__.py, they are removed from other places in the code.
  • renamed problem_parameters() to get_problem_parameters() in all problems. The function no longer changes NS_parameters, but rather returns a dictionary problem_parameters with the default parameter values.
  • replaced NS_parameters with problem_parameters in the main executable
  • The post_import_problem() function now separates NS_namespace and problem_parameters. Previously everything was merged into one dictionary d. Anything in d was copied into the namespace of the main executable
  • replaced any arguments like **vars() or **globals() with the dictionaries NS_parameters and problem_parameters

@mikaem
Copy link
Owner

mikaem commented Mar 3, 2022

Hi @flabowski,
Sorry about the late reply. The effort is much appreciated, and I understand your arguments perfectly. I actually considered removing the global dictionary earlier when I was more actively developing this code. However, I decided against it then since I had grown accustomed to the current version, and I like to scare matlab programmers with global variables:-)

Unfortunately, though, this PR is much too big to simply incorporate into master. It would destroy too many applications, especially for some of my master students, and some of the tools that have been further developed for FSI, that are using oasis. A new version could perhaps be considered, but then again I'm not sure there is good reason to make such big changes for a code that depends on the now legacy FEniCS? I would probably be much more interested in a completely new implementation, using the algorithms in the current code (described in the Oasis paper), and that make use of the new and still much developed FEniCSx. Have you considered that?

@flabowski
Copy link
Author

Hei, i agree, i can see that this would be a whole new project. I am not sure if i want to commit to FEniCSx and a re-implementation of all the solvers. I need only the IPCS_ABCN only for my project.

@flabowski flabowski closed this Mar 4, 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.

2 participants