Skip to content

Conversation

@nautolycus
Copy link
Collaborator

Updated definitions relating to the bond valence model and added 'valence_units' to the units list.

Follows discussion in Issue #62

  • I have added _valence_param.citation_id with some discussion of how this complements the VALENCE_REF category;
  • I set the _enumeration.range for _geom_bond.valence to 0.0:10.0
  • I haven't made any reference to the `9' convention for oxidation states that @vaitkus mentioned. I'm reluctant to hard-wire such a convention into a standard definition. I will contact the current maintainer of the valence parameters set to see if they are open to using '?' instead.

Copy link
Collaborator

@vaitkus vaitkus left a comment

Choose a reason for hiding this comment

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

Looks good overall, left a few comments.

Copy link
Collaborator

@vaitkus vaitkus left a comment

Choose a reason for hiding this comment

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

Thank you for the work, I am generally happy to merge this as is.

Would it be useful to leave issue #62 open for the time of being with the "Update specification" label as a reminder that [1] should be mentioned in the commentary chapter in Volume G?

[1] https://www.iucr.org/resources/data/datasets/bond-valence-parameters

@nautolycus
Copy link
Collaborator Author

Would it be useful to leave issue #62 open for the time of being

Yes, that would be fine - I've noted a number of issue sthat have been left open for this purpose, and will try to address those soon.

@vaitkus vaitkus merged commit 4532653 into COMCIFS:master Nov 9, 2025
3 checks passed
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.

3 participants