Skip to content

Conversation

@tjjarvinen
Copy link
Contributor

@tjjarvinen tjjarvinen commented Oct 22, 2025

The main update is that the old FlexibleSystem additions are removed and AtomsSystems structures are used. Most of the utils used in AtomsBuilders are implemented in AtomsSystems and thus using them is better than maintaining them here.

This update does not change how crystals are build. The only changes are use AtomsSystems structures and removing utilities that are now implemented in AtomsSystems.

Changes

repeat

repeat is defined in Julia.Base and use of it here for FlexibleSystem is type piracy (neither repeat or FlexibleSystem is defined in AtomsBuilder), so I removed it. AtomsSystem has repeat defined for its structures and is thus not type piracy.

The short cut of using * for repeat e.g. bulk(:Cu) * 3 does not work. This kind of use was discussed during AtomsSystems review process and was dropped due to ambiguity. It would also be type piracy to implement it in AtomsBuilder.

# old use
sys = bulk(:Cu) * 3

# new and old use that works
sys = repeat( bulk(:Cu), 3)

This is the biggest change and thus needs a major version bump.

rattle

AtomsSystems has rattle_positions implemented with AtomsBase.set_position! and thus works with all AB structures that implement the given command.

Note that AtomsSystems command uses _positions suffix. This is more explicit and gives room for future rattle_cell command etc.

# old use
rattle( sys, 0.5)
rattle!( sys, 1.0u"Å")

# new use
rattle_positions( sys, 0.5)
rattle_positions!( sys, 1.0u"Å")

union

Combine two systems with union is problematic, firstly it is type piracy and secondly it can have unexpected results due to general nature of union (it has implementations for iterators etc.). Thus AtomsSystems uses add_systems function to be more explicit and avoid other issues

# old use
sys12 = union( sys1, sys2 )

# new use
sys12 = add_systems( sys1, sys2 )

randz!

randz! works as usual, but now uses random_species! on the background. random_species! uses StatsBase.sample for more control on sampling and is an additional implementation in AtomsBuilder. It can be used in the following way

sys = repeat(bulk(:Ti), 3)

spc = [:Ti, :Al]  # species to sample from

# Create weights for each species
w = FrequencyWeights([1, 3])
w = ProbabilityWeights([0.2, 0.8])
w = Weights([0.2, 0.8]) # try to resolve to probabilities
# or any other Weights type in StatsBase

random_species!( sys, spc, w )

@cortner
Copy link
Member

cortner commented Oct 23, 2025

  • I'm strongly opposed to moving repeat, *, union etc utilities out of AtomsBuilder. It may technically be type piracy, but because AtomsBase and AtomsBuilder are jointly maintained I don't see this as an issue and I dislike development of this functionality moving into a package for specific datatypes as opposed to remaining datatype generic.

  • I strongly dislike add_systems as it is not idiomatic language. I don't understand why renaming something generates ambiguity. union is in my view the correct term to use here and one simply needs to document what the operation does.

  • I don't have a strong view on renaming of the rattle functions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants