A library for solving differential equations using neural networks based on PyTorch, used by multiple research groups around the world, including at Harvard IACS.
[ ] how the library is broken into submodules and classes: Submodules like ode, pde, pde_spherical, tdim(time-dependent PDEs under development) are not mutually exclusive. Many of the functions and classes share very similar logic (e.g. ode.solve_system and pde.solve2D_system). This amplifies the work needed to make changes (e.g. every time I change pde.solve2D_system, I almost always need to make similar changes to ode.solve_system as well). We may want to extract these logics.
[ ] function signatures are not carefully thought out:
keywords whose values can potentially contradict each other, e.g. What if in pde.solve2D_system, the domain indicated by xy_min and xy_max is different from that indicated by train_generator?
keywords that come from different abstraction level, e.g. in pde.solve2D_system, there are keywords that come from the realm of differential equations (higher level) and keywords that come from the realm of deep learning (lower level). Maybe it's just me but that feels uncomfortable.
keywords that always appear together, e.g. nets and single_net in pde submodule. This suggests that they really should be made into one parameter object.
the names and order of keywords are not consistent across the package.
[ ] conversion between numpy.array and torch.tensor: numpy.array and torch.tensor are converted to each other everywhere and I find myself checking whether a sequence is a numpy.array or torch.tensor all the time. This makes working on / using the code less pleasant. Since the only reason numpy.array is there is because of matplotlib.pyplot and numpy.linalg, we should try to limit the use of numpy.array only to those sections and use torch.tensor in other places. Also, numpy.array and torch.tensor use different default precision, conversion can potentially introduce error.
[ ] the use of torch.tensor:
I was not mindful of the memory footprint of torch.tensor when I wrote the code, there may be cases where requires_grad is set to True when it really should be False. there may also be cases where we can reuse a tensor yet it is re-created.
I was not mindful when reshaping torch.tensor. I'm not sure the impact on performance.
[ ] naming things:
non-standard format: function names should not have uppercase letters (e.g.solve2D); upper case variable names are not reserved for constants and enumerations.
names that give false impressions: e.g. ExampleGenerator is not a python generator.
confusing names: e.g. What is an additional_loss? What is an FCNN? Why are there t_0 and t_1 in ode.DirichletBVP which has nothing to do with time?
[ ] the use of strings as categorical variables: should have used Enum instead, in other cases (e.g. the as_type keyword of pde.Solution.__call__) should have used a bool instead.
Things that should be cleaned up:
ode,pde,pde_spherical,tdim(time-dependent PDEs under development) are not mutually exclusive. Many of the functions and classes share very similar logic (e.g.ode.solve_systemandpde.solve2D_system). This amplifies the work needed to make changes (e.g. every time I changepde.solve2D_system, I almost always need to make similar changes toode.solve_systemas well). We may want to extract these logics.pde.solve2D_system, the domain indicated byxy_minandxy_maxis different from that indicated bytrain_generator?pde.solve2D_system, there are keywords that come from the realm of differential equations (higher level) and keywords that come from the realm of deep learning (lower level). Maybe it's just me but that feels uncomfortable.netsandsingle_netinpdesubmodule. This suggests that they really should be made into one parameter object.numpy.arrayandtorch.tensor:numpy.arrayandtorch.tensorare converted to each other everywhere and I find myself checking whether a sequence is anumpy.arrayortorch.tensorall the time. This makes working on / using the code less pleasant. Since the only reasonnumpy.arrayis there is because ofmatplotlib.pyplotandnumpy.linalg, we should try to limit the use ofnumpy.arrayonly to those sections and usetorch.tensorin other places. Also,numpy.arrayandtorch.tensoruse different default precision, conversion can potentially introduce error.torch.tensor:torch.tensorwhen I wrote the code, there may be cases whererequires_gradis set toTruewhen it really should beFalse. there may also be cases where we can reuse a tensor yet it is re-created.torch.tensor. I'm not sure the impact on performance.solve2D); upper case variable names are not reserved for constants and enumerations.ExampleGeneratoris not a python generator.additional_loss? What is anFCNN? Why are theret_0andt_1inode.DirichletBVPwhich has nothing to do with time?as_typekeyword ofpde.Solution.__call__) should have used a bool instead.