arfc / moltres

Repository for Moltres, a code for simulating Molten Salt Reactors
GNU Lesser General Public License v2.1
66 stars 41 forks source link

Update Doxygen html documentation output on Moltres #148

Closed smpark7 closed 3 years ago

smpark7 commented 3 years ago

Closes #146

This PR updates the Doxygen html documentation on Moltres, which is hosted here and was last updated in 2017.

This PR also corrects LaTeX syntax typos in the documentation in FissionHeatSourceAux.h and DiffusionNoBCBC.h.

yardasol commented 3 years ago

@smpark7 Do you have any recommendations on how I can review this PR? There are over 1000 files edited and I have no idea where to start.

katyhuff commented 3 years ago

Something is failing in the MOOSE CIVET tests. Unclear to me why. It seems to be missing user info necessary for cleaning up the ubuntu 16 instance?


...
//: cp /home/moosetest/singularity/start_moosebuild.sh /home/moosetest/singularity/.civet_buildq11_start_moosebuild.PfRml
//: source /home/moosetest/singularity/.civet_buildq11_start_moosebuild.PfRml
whoami: cannot find name for user ID 1002: Connection refused
/home/moosetest/singularity/.civet_buildq11_start_moosebuild.PfRml: line 682: [: =: unary operator expected
//: Instructing Singularity to use environment set forth by Civet
//: mkdir -p /tmp/Ubuntu-16
//: chmod o-rwx /tmp/Ubuntu-16
//: mapping /tmp/Ubuntu-16 as /tmp within container
whoami: cannot find name for user ID 1002: Connection refused
/home/moosetest/singularity/.civet_buildq11_start_moosebuild.PfRml: line 323: [: =: unary operator expected
//: Using moose-environment release: 6f3c438e838564d48bf591191986ef747f50c8e1
//: ARCH=Ubuntu-16.04
//: BUILD_DATE=20191104
//: PR_VERSION=https://github.com/idaholab/package_builder/pull/213
//: rm /home/moosetest/singularity/.civet_buildq11_start_moosebuild.PfRml
//: cp /tmp/tmp0yJ9cK /tmp/Ubuntu-16/
/tmp/: /opt/singularity/bin/singularity exec --no-home /home/moosetest/singularity/containers/Ubuntu-16.simg /tmp/tmp0yJ9cK
ERROR  : Failed to obtain user identity information
ABORT  : Retval = 255
/tmp/: rm -rf /tmp/Ubuntu-16```
smpark7 commented 3 years ago

Yea I'm not sure why the test failed in such a way either. The error message seems to suggest it wasn't anything in Moltres that triggered it.

@yardasol The only real changes in documentation are in FissionHeatSourceAux.h and DiffusionNoBCBC.h (fixing typos). All other changes seem like formatting changes from using a newer Doxygen version (1.8.13 -> 1.8.17). I think it is safe to just check that you can reproduce the same html output from running Doxygen 1.8.17 with the Doxygen config file in the base Moltres directory. And we can check the documentation for each individual kernel as we work on Issue #133 in the near future. @katyhuff would you agree?

katyhuff commented 3 years ago

We definitely need the tests to pass.

On Fri, Jan 29, 2021 at 3:43 AM Sun Myung Park notifications@github.com wrote:

Yea I'm not sure why the test failed in such a way either. The error message seems to suggest it wasn't anything in Moltres that triggered it.

@yardasol https://github.com/yardasol The only real changes in documentation are in FissionHeatSourceAux.h and DiffusionNoBCBC.h (fixing typos). All other changes seem like formatting changes from using a newer Doxygen version (1.8.13 -> 1.8.17). I think it is safe to just check that you can reproduce the same html output from running Doxygen 1.8.17 with the Doxygen config file in the base Moltres directory. And we can check the documentation for each individual kernel as we work on Issue #133 https://github.com/arfc/moltres/issues/133 in the near future. @katyhuff https://github.com/katyhuff would you agree?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/arfc/moltres/pull/148#issuecomment-769697050, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADAFK74E3AWFU2EVQHEMGDS4J7NXANCNFSM4WXLPFSA .

-- http://katyhuff.github.com

smpark7 commented 3 years ago

I pushed a commit to trigger a new round of CIVET tests. This round of tests has already passed the "Clean machine" Precheck checkpoint where we encountered the previous failure, so it looks good so far.

katyhuff commented 3 years ago

Excellent. Now my only concern is that there tons of files. I have a couple of questions: 1) Is this the right branch to submit these changes to? (with documentation, it's common to have the source changes in one branch, with the built files deployed in another branch. I forget how we did it here.) 2) Are all of these files definitely necessary to include in the repo? (it's just a lot of them.)

On Fri, Jan 29, 2021 at 7:49 AM Sun Myung Park notifications@github.com wrote:

I pushed a commit to trigger a new round of CIVET tests. This round of tests has already passed the "Clean machine" Precheck checkpoint where we encountered the previous failure, so it looks good so far.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/arfc/moltres/pull/148#issuecomment-769815783, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADAFKZZ5ZI3LD4TKO2LNK3S4K4IBANCNFSM4WXLPFSA .

-- http://katyhuff.github.com

smpark7 commented 3 years ago

I agree that it is a ton of files and probably not in the right branch. We could set up a gh-pages branch that automatically compiles the docs every time there's a change.

The alternative would be perhaps hosting the files on the arfc.github.io repo

katyhuff commented 3 years ago

Created issue #149 to handle the question of which branch this belongs in.