refactoring partitioned-heat-conduction/nutils #295
Merged
Conversation
This file contains 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
This patch removes the leading dashes for cli arguments in the run script, which were always optional but have recently been forbidden. The changes are compatible with all versions of nutils.
This patch disables rich output by default, in order for standard output logging to co-exist nicely with precice's log messages.
This patch removes the droptol argument in the projection of the initial condition, which is a full domain projection that should include all degrees of freedom.
This patch simplifies the boundary constraints by using uexact.
This patch replaces sample.asfunction by sample.basis combined with an argument for readdata. While both constructions yield the same object internally (asfunction uses basis under the hood), the new form makes it possible to construct functionals ahead of time and benefit from cached data structures, rather that recreate them at every iteration (see next commit).
|
Thanks for the refactoring @gertjanvanzwieten! Is this ready for reviewing? |
|
I noticed that autopep8 isn't happy, in part because of the double space indent on line 101 to avoid double indentation of 103-149, but I can make it compliant if you prefer. Otherwise it's ready for review! |
|
Please be aware that Nutils may trigger an issue unrelated to this PR that we are currently in the process of fixing. The problem is exacerbated in the development version but exists in stable as well, unnoticed until now. |
IshaanDesai
reviewed
Sep 30, 2022
IshaanDesai
reviewed
Sep 30, 2022
IshaanDesai
reviewed
Sep 30, 2022
IshaanDesai
approved these changes
Sep 30, 2022
This patch defines res and sqr outside the time loop, so that the only things changing per timestep are the arguments (including readdata). The resulting code is both cleaner and faster due to reuse of cached structures.
This patch simplifies the read and write instructions by sticking the arguments inside functools.partial, leaving a more readable remainder of the code.
This patch replaces the projection matrix by a functional based equivalent that is a bit more ideomatic nutils. It also fixes a consistency issue at the ends of the boundary and adds an elaborate comment to explain the construction.
This patch bundles the checkpointed state in a single `checkpoint' tuple and unifies output if the initial condition and following time steps.
This patch makes all parameters into arguments of main so that they can be configured from the command line (though not via run.sh). This makes it possible, for example, to experiment with different timesteps on either half of the simulation.
gertjanvanzwieten
force-pushed
the
phc-nutils
branch
from
eec74b6
to
b42539d
Compare
|
I removed the shifted domain (which was not compatible with the non-nutils implementations of this tutorial that I completely forgot about) and the nested loop, so I'm hoping the new version fixes the remaining issues of this PR. |
IshaanDesai
reviewed
Oct 12, 2022
IshaanDesai
reviewed
Oct 12, 2022
IshaanDesai
reviewed
Oct 12, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Some suggested changes to the heat.py script to make it more efficient, more correct, and hopefully easier to read, while remaining backwards compatible with nutils 6 as well as forwards compatible with nutils 8. The commit messages explain the reasoning behind the individual modifications.