JuliaMolSim / Molly.jl

Molecular simulation in Julia
Other
372 stars 51 forks source link

Fixed visualization issue with Makie.jl #63

Closed chemicalfiend closed 2 years ago

chemicalfiend commented 2 years ago

Changed the markerspace parameter on the scatter function to feed makie the required data type (looks like the TypeSpace style has been removed from Makie? I can't find the exact point in the version control where this happened but seems to be the case). Also added LinearAlgebra since norm wasn't working on the diatomics simulation without it.

codecov[bot] commented 2 years ago

Codecov Report

Merging #63 (561b42f) into master (e526479) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #63   +/-   ##
=======================================
  Coverage   86.74%   86.74%           
=======================================
  Files          28       28           
  Lines        2912     2912           
=======================================
  Hits         2526     2526           
  Misses        386      386           
Impacted Files Coverage Δ
src/makie.jl 0.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e526479...561b42f. Read the comment docs.

jgreener64 commented 2 years ago

Great, I'll have a look next week.

jgreener64 commented 2 years ago

Fixes the issue but the diatomic case at https://github.com/JuliaMolSim/Molly.jl/blob/master/test/simulation.jl#L204-L206 still fails, I think due to the other call to SceneSpace at https://github.com/JuliaMolSim/Molly.jl/blob/master/src/makie.jl#L74?

using LinearAlgebra should not be required as it is already called at https://github.com/JuliaMolSim/Molly.jl/blob/master/src/Molly.jl#L26. Are you running this as part of a dev Molly source tree? That is the best way to develop as it lets you run the tests too.

chemicalfiend commented 2 years ago

I'll look into it. I haven't been using a dev Molly source tree but I will to take care of the tests.

jgreener64 commented 2 years ago

:+1: