Skip to content

Add Spin/Charge to KNOWN_INPUTS#189

Open
JonathanSchmidt1 wants to merge 13 commits intometatensor:mainfrom
JonathanSchmidt1:spin_charge_inputs
Open

Add Spin/Charge to KNOWN_INPUTS#189
JonathanSchmidt1 wants to merge 13 commits intometatensor:mainfrom
JonathanSchmidt1:spin_charge_inputs

Conversation

@JonathanSchmidt1
Copy link
Copy Markdown

@JonathanSchmidt1 JonathanSchmidt1 commented Mar 28, 2026

Adds Spin/Charge to KNOWN_INPUTS (ase integration in another PR)
and a respective test/docs.
_check_outputs also seems to be used for checking inputs with check consistency so I asked claude to add respective checks ...
Not sure if we want separate docs for inputs but i imagine also with support for FlashMD and momenta inputs it might be nice to separate it.

Contributor (creator of pull-request) checklist

  • Tests updated (for new features and bugfixes)?
  • Documentation updated (for new features)?
  • Issue referenced (for PRs that solve an issue)?

Reviewer checklist

  • CHANGELOG updated with public API or any other important changes?

@JonathanSchmidt1 JonathanSchmidt1 requested a review from Luthaf March 28, 2026 16:44
JonathanSchmidt1 and others added 3 commits March 28, 2026 17:59
These validate the TensorMap metadata for system-level charge and spin
inputs when check_consistency=True: single block, per-system samples,
no components, correct property name, no gradients.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Luthaf
Copy link
Copy Markdown
Member

Luthaf commented Mar 31, 2026

Not sure if we want separate docs for inputs but i imagine also with support for FlashMD and momenta inputs it might be nice to separate it.

There is no distinction between stuff that can be used an inputs and stuff that can be produced as outputs, and everything lives in output/<name>.rst for historical reason, and might move with the next breaking change.

Charge
^^^^^^

The total charge of the system is associated with the ``"charge"`` name, and
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having separate charge and charges concept, I think we should unify both (and likely rename everything to be singular, but I'll do it in a separate PR)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then we can use per_atom=True/False (soon sample_kind="system" | "atom" to distinguish between the two cases

Spin
^^^^

The spin multiplicity of the system is associated with the ``"spin"`` name,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be named spin_multiplicity? Or is there no chance that we will want both spin and spin_multiplicity in the future?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we will have less trouble if we are compatible with everbody else. Once we have spin atomic spins will be different anyway and we could use magnetization for other purposes if we ever need it

@JonathanSchmidt1
Copy link
Copy Markdown
Author

Not sure if we want separate docs for inputs but i imagine also with support for FlashMD and momenta inputs it might be nice to separate it.

There is no distinction between stuff that can be used an inputs and stuff that can be produced as outputs, and everything lives in output/<name>.rst for historical reason, and might move with the next breaking change.

Thank you for the review. Than I will put everything back into outputs.

JonathanSchmidt1 and others added 3 commits April 8, 2026 14:52
Drop charge as a standard name. Rename spin to spin_multiplicity
everywhere (C++ registration, validation, tests, docs). The ASE
info key remains atoms.info["spin"] for user convenience.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Format long ModelOutput line per ruff. Add spin_multiplicity to
QUANTITY_DIMS in units.cpp as a dimensionless quantity to avoid
unknown-quantity warning.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@JonathanSchmidt1 JonathanSchmidt1 mentioned this pull request Apr 8, 2026
4 tasks
@JonathanSchmidt1
Copy link
Copy Markdown
Author

@Luthaf I removed the charge part and switched to spin-multiplicity as requested. Please let me know if it looks good now.

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