search

mabarnes / moment_kinetics

Other
2 stars 4 forks source link

Geometry upgrade separate packages #179

Closed mrhardman closed 8 months ago

mrhardman commented 8 months ago

Pull request to merge my own merge of geometry-upgrade and the branch which restructures the packages which make up moment_kinetics.

Made for comparison to try to find errors.

mrhardman commented 8 months ago

My main observation doing this merge was that catching errors in the merge of the external manufactured solutions module was annoying due to unclear error messages. Perhaps because I am building manually, I had to iterate several times to get both Symbolics and MPI to build consistently.

johnomotani commented 8 months ago

My main observation doing this merge was that catching errors in the merge of the external manufactured solutions module was annoying due to unclear error messages. Perhaps because I am building manually, I had to iterate several times to get both Symbolics and MPI to build consistently.

That's odd, I haven't had similar problems. Do you remember what it was that didn't work? We could maybe improve the manual setup instructions. To be honest, I hadn't really expected anyone to try to follow them! Hopefully the setup script is a record of one set of operations that does work!

mrhardman commented 8 months ago

At least in the most recent example, when I used git pull to update the local repo I had to reinstall MPI and Symbolics in the package manager before I could run a simulation. In the end everything worked, but I had to know that that was the problem that needed fixing!

mrhardman commented 8 months ago

P.S. In principle, I would want to support the manual setup instructions in addition to your scripts. In the event that your script fails to work it would be much easier for a new user to follow manual setup instructions (and potentially write their own script) than to back out from your script what they need to do.

johnomotani commented 7 months ago

At least in the most recent example, when I used git pull to update the local repo I had to reinstall MPI and Symbolics in the package manager before I could run a simulation. In the end everything worked, but I had to know that that was the problem that needed fixing!

Right, I think this is a documentation issue. BTW, if you try to use MMS features without having installed Symbolics, you should have got an error message suggesting that you install Symbolics. If that message didn't appear, that's a bug I should fix...