fix: remove unnecessary casts of atom indices

[ezavod]

Some small changes:

• Atom indices are usually given as u64 but some functions expected usize and then immediately cast the parameter to u64.
• clippy got an update: stutter is deprecated
• Since most structs contain the module name and the name is somewhat set, because this is mostly just a wrapper, IMHO it is reasonable to allow module_name_repetitions globally, isn't it?

BTW: Should we run rustfmton all the files?

[Luthaf]

Atom indices are usually given as u64 but some functions expected usize and then immediately cast the parameter to u64.

The idea here was that the use of u64 is an implementation detail (the uint64_t are created at the C API level to avoid dealing with different size of size_t), so the values should always fit in an usize, and dealing with usize is nicer on the rust side since they can be used to index vectors and other data.

BTW: Should we run rustfmton all the files?

The last time I tried, rustfmt output was really bad, and not configurable on stable. Do you know if this changed? I'll try to give it a look too.

[ezavod]

dealing with usize is nicer on the rust side since they can be used to index vectors and other data.

Oh, I see. However there seems to be a general type mismatch then: a lot of other functions either return u64 or expect this type as argument for an atom index. So I regularly need a lot of u64 <-> usize conversion which doesn't feel right.
See e.g.:

• fame.size() returns u64, so when iterating over all atoms I get u64 as indices.
• frame.atom(index) expect an index as u64.
• Matches are explicitly documented as arrays of u64.

Is making all index related data types usize a better solution?

The last time I tried, rustfmt output was really bad, and not configurable on stable. Do you know if this changed?

I haven't had any issues recently and I'm pretty sure that it works on stable now (except some minor features like formatting strings). But this is actually more a suggestion than a thought through plan.

[Luthaf]

Is making all index related data types usize a better solution?

I would rather do this, yes 😃

[ezavod]

Ok, then this won't be as quick and easy as I thought 😅 Will look into this...

so the values should always fit in an usize

Not sure if this is valid the other way around. Can't find any guarantee that usize fits always in u64. And it exists try_from, so I would not thing that we could assume no overflows :(

[ezavod]

Not sure how to handle this properly, this feels unidiomatic. Any suggestions?

[Luthaf]

Yeah, I think this is why the API had mixed usize/u64 in the first place: we need to allocate new memory for bonds/angles/... to transform them from u64 to usize. This is a bit unfortunate, and I don't know how to improve it here.

---

But the way you are transforming from Vec<[u64; 2]> to Vec<[usize; 2]> looks good to me. It might be possible to optimize it by using Vec::reserve instead, but this iterator-based code should already be fast enough.

[Luthaf]

I would avoid naming the variable in map tuple, since it is not a tuple in Rust sense. How about bond?

[ezavod]

Renaming makes sense, done.
I just learned, that collectover iterators that are sized or owned tends to be quite performant. So I guess we wouldn't improve with Vec::reserve.

[ezavod]

There are a lot of clippy warnings because of u64 -> usize. Will add allow annotation later.

[Luthaf]

Not sure if this is valid the other way around. Can't find any guarantee that usize fits always in u64. And it exists try_from, so I would not thing that we could assume no overflows :(

What I mean by that is that all u64 values created by chemfiles will fit in an usize (even if usize is 32-bit), because they are all created from size_t values on the C++ side. So in this specific case we don't have to worry about overflow.

In the general case, u64 can overflow usize on platforms that use 32 or 16-bit indexing.

[ezavod]

I'm not worried about casting chemfiles u64 to usize. Casting usize to u64 whenever we get usize as function argument might overflow.
Currently rust doesn't support any targets with word length > 64, but this could hypothetically change in the future. There is no guarantee that usize fits in u64.
But this is probably not much of an issue :D