search

libAtoms / ExtXYZ.jl

Extended XYZ read/write support for Julia
MIT License
13 stars 6 forks source link

show_system for AtomsBase interface #21

Closed JanHab closed 2 years ago

JanHab commented 2 years ago

Hello. I think it would be nice to add a Base.show function similar to the way it is done in AtomsBase. In this routine the atomic_symbols, the box, and if all boundary_conditions are the same, the boundary_conditions are showed. Additionally, at the beginning, there is written whether the system is a FlexibleSystem or a FastSystem. If one just creates an atomic_system in AtomsBase, there is a FlexibleSystem created.

I am not complete sure if ExtXYZ would create a system similar to a FlexibleSystem or a FastSystem?

I would like to add this function, any thoughts on this?

jameskermode commented 2 years ago

Happy to add this. The idea of the ExtXYZ.Atoms struct is that it is both flexible and fast, since the type information for all properties is included in the parametric type definition.

JanHab commented 2 years ago

Okay so that means we would skip the part where it is showed which kind of system it is?

jameskermode commented 2 years ago

Yes that sounds right - I should have a look at what the text output looks like, but if you are willing to implement this I'll try it out

JanHab commented 2 years ago

In AtomsBase they do this for both, FastSystem and FlexibleSystem, by using their routine show_system. I think it is possible to do the same here.

function Base.show(io::IO, system::Atoms) print(io, "System") AtomsBase.show_system(io, system) end

seems to work. In AtomsBase the "System" in the print is "FastSystem" or "FlexibleSystem", I am not sure about the best name for it in this case.

In a case where I tried this the system was showed like this: System(Mn₃Si, AtomsBase.Periodic, box=[[3.99986972, 0.0, 0.0], [1.9999348600000004, 3.463988789348149, 0.0], [1.9999348600000004, 1.1546629297827167, 3.2658799505363403]]u"Å")

If i am not forgetting anything important here I think it could work with this code example, depending on how to call the system.

jameskermode commented 2 years ago

Should the name not just be Atoms() to match the struct name?

From: Jan Habscheid @.> Sent: Monday, September 26, 2022 5:16:28 PM To: libAtoms/ExtXYZ.jl @.> Cc: James Kermode @.>; Comment @.> Subject: Re: [libAtoms/ExtXYZ.jl] show_system for AtomsBase interface (Issue #21)

In AtomsBase they do this for both, FastSystem and FlexibleSystem, by using their routine show_system. I think it is possible to do the same here.

function Base.show(io::IO, system::Atoms) print(io, "System") AtomsBase.show_system(io, system) end

seems to work. In AtomsBase the "System" in the print is "FastSystem" or "FlexibleSystem", I am not sure about the best name for it in this case.

In a case where I tried this the system was showed like this: System(Mn₃Si, AtomsBase.Periodic, box=[[3.99986972, 0.0, 0.0], [1.9999348600000004, 3.463988789348149, 0.0], [1.9999348600000004, 1.1546629297827167, 3.2658799505363403]]u"Å")

If i am not forgetting anything important here I think it could work with this code example, depending on how to call the system.

— Reply to this email directly, view it on GitHubhttps://github.com/libAtoms/ExtXYZ.jl/issues/21#issuecomment-1258292166, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAIAKTBHORFZDOS5F5OI7TTWAHD5ZANCNFSM6AAAAAAQVAQSJA. You are receiving this because you commented.Message ID: @.***>