MOLE.jl: Refactoring in Opearors and BCs Modules

[valeriabarra]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Optimization
• Example
• Documentation

Description

This PR refactors the current MOLE.jl into (sub)Modules. In the existing version, only the superModule MOLE existed and all operators and boundary condition implementation files were homed all in the same MOLE.jl/src/ directory.

• This PR refactors and reorganizes these files in Modules that represent the logical units of code (one for Operators and one for BCs).
• The same organization structure is applied to both the MOLE.jl/src/ and MOLE.jl/test/ subdirectories.
• Stale comments have been cleared from the MOLE.jl/test/runtests.jl file

Related Issues & Documents

• Closes Refactor existing MOLE.jl directory structure to match proposed directory layout #280

Added/updated tests?

• Yes
• No, and this is why: please replace this line with details on why tests
  have not been included
• I need help with writing tests

Read Contributing Guide and Code of Conduct

• I have read and followed the Contributing Guide and the Code of Conduct

[mathgeek31415]

Hi @valeriabarra , thanks for this refactoring suggestion! I agree with your changes, and I think this will help keep the code more organized.

One comment: currently, with the changes as-is, the "elliptic1D.jl" file won't run, specifically because the lap and robinBC functions are "not recognized". I noticed that in the updated MOLE.jl file, the following line was removed

export div, grad, lap, robinBC

which means that we would need to explicitly call MOLE.lap and MOLE.robinBC. If I make these changes to the elliptic1D example (lines 16 and 21), then the example runs fine. (Alternatively, adding back the export statement to MOLE.jl allows the example to run without needing to prefix the function calls with MOLE.)

What is the standard practice? Should we require that the module name be called before the function, or should we keep the export statement?

Thank you.

[valeriabarra]

Hi @mathgeek31415 , thank you very much for your review. Yeah, you are right that I had forgot to run the example (I had focused on the tests first). In my commit c9cb544 I addressed your concerns. I decided to still not use the export keyword. Usually, for common names that can easily clash with other common mathematical functions (such as "derivative", "divergence", "gradient", etc) it is preferred to keep the namespace to avoid confusion (check this Julia Lang Doc page). I have now tested the example as well as run the test suite and everything should be working fine. Thanks!

[mathgeek31415]

Everything looks good here! The example file (elliptic1D.jl) and the unit tests all seem to be working correctly. These changes are approved for merging.

[valeriabarra]

@jbrzensk we need at least one approving reviewer with write access to be able to merge. Thanks