🐛 Incorrect constructor for AtomsCalculatorsModel with PeriodicCell

[Alexsp32]

No description provided.

[Copilot]

Pull request overview

Fixes an incorrect AtomsCalculatorsModel constructor path when working with periodic cells, and expands tests to cover both infinite and periodic cell structures.

Changes:

• Update AtomsCalculatorsModel constructors to always set ndofs to 3 instead of conditionally using the cell object.
• Expand the AtomsCalculators test to run against both InfiniteCell() and PeriodicCell(...).
• Tighten Subsystem.indices typing and add a Codecov upload step in CI.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
test/runtests.jl	Adds coverage for AtomsCalculatorsModel using both infinite and periodic cells.
src/subsystems.jl	Changes Subsystem.indices type and constructor signature/normalization logic.
src/classical_models/atomscalculators.jl	Fixes the ndofs constructor argument to always be 3.
.github/workflows/CI.yml	Adds Codecov upload step (currently without generating lcov.info).

Comments suppressed due to low confidence (1)

src/subsystems.jl:33

• The Int normalization indices = [indices:indices] creates a Vector{UnitRange{Int}}, which is incompatible with the indices::Union{Vector{Int}, Colon} field type and will fail at construction. This should produce a Vector{Int} (e.g., a single-element vector or collect(indices:indices)) to match the struct field type and downstream indexing assumptions.

	if isa(indices, Int)
		indices = [indices:indices]
	elseif isa(indices, UnitRange{Int})
		indices = collect(indices)
	end

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Subsystem(model) (and calls that pass Colon() explicitly) will now throw a MethodError because the constructor only accepts Int/UnitRange{Int} while the default value is : (a Colon). Consider widening the signature to also accept Colon (and likely AbstractVector{<:Integer}) so existing CompositeModel(...)=Subsystem.(models, Colon()) and Subsystem(model) keep working.

[Copilot]

The workflow uploads lcov.info to Codecov, but there is no step enabling Julia test coverage or generating an LCOV report in this job. As-is, this will likely fail (missing file) or upload nothing; consider enabling coverage in julia-runtest and/or adding a coverage-to-lcov conversion step, and use the action input expected for multiple files (commonly files:) if needed.

[htsnowden]

All looks good