Conversation
…t/JPEC into sing_unit_tests
There was a problem hiding this comment.
I don't think so- I think this one got uploaded by accident. Oops
|
I think a lot of this can be combining the.dat files into a single HDF5 - this will mean only one data file needs to be added and we can also remove some of the read in functions within the actual testing script, since it can just be like |
|
We also have to make sure that the compile test passes! It looks like the workflow halts when we add a package to the repo, I will need to figure that out. I will do so over the weekend. |
test/runtests.jl
Outdated
| @@ -1,5 +1,4 @@ | |||
|
|
|||
|
|
|||
| using LinearAlgebra, BandedMatrices, Random, Test, DelimitedFiles, Printf #TODO: clean up imports - do we need all of these here? | |||
There was a problem hiding this comment.
I believe this is causing the issue that @matt-pharr raised with the build failing - the JPEC package isn't using BandedMatrices (it's not in the Project.toml), so I would expect the build to rightfully fail here. So that part of this call should be removed. Also, test is duplicated, so probably that too. And for the rest of them, if they're not in the Project.toml, we can't import them here I don't think, so they either need to be added or alternatives found. I think for the ones that are in there (LinearAlgebra, Printf), doing a using JPEC gives them to you for free without needed a using statement here. I think delimited files won't be needed if we switch to HDF5.
There was a problem hiding this comment.
That sounds good! I will look at cleaning up the imports and making sure the Project.toml matches the develop branch later this week
… sing_unit_tests to make sure we are testing the most recent version of the code
…th current develop branch. The fortran vacuum unit tests are causing a seg fault right now though
…il still. Seems to be the includes
…I think it is the includes
…ludes into sing test file. Probably gonna need to do a bigger fix
…ackage so it should pass the tests now
…g_unit_tests to keep branch up to date with develop
|
@mfairborn23 what is the status here? |
|
@logan-nc I have to double check this branch again but at least with the previous version of the code I believe that there were unit tests for most functions. My new branch has some additional unit tests for the kinetic versions of the code as well. |
|
please ping me and @jhalpern30 when this is ready for review. if not soon, then we should just close the pr. it's going to become too stale and out of date |
… the unit tests run. We can probably review and merge this and when I am finished with the kinetic part of the code, we have a few more unit tests there we can merge in here.
|
@jhalpern30 and @logan-nc I think we need to figure out what this "Documentation" issue is, then this can be reviewed. I attempted to merge develop into it and have confirmed the unit tests are still working. There will be some more unit tests we can add when we merge the kinetic branch in because I wrote a few more there, but I believe this is a good start. Once I am finished with the kinetic part, I will review the code and ensure there aren't any other functions we should make unit tests for. |
|
@matt-pharr any chance you can make progress on the documentation issue? |
4 participants
This needs clean up and some more development before it is merged with develop, but here is an early version of the sing_der unit test. We still need to add unit tests for the other sing.jl functions and clean up the sing_der one because there is currently a lot of clutter with it