electronic-structure / SIRIUS

Domain specific library for electronic structure calculations
BSD 3-Clause "New" or "Revised" License
115 stars 40 forks source link

add size check / simplify vec_from_str #983

Closed simonpintarelli closed 2 months ago

simonpintarelli commented 2 months ago

Use std::stof, std::stod in vec_from_str, these function throw an error if an element cannot be converted. Afaik the istream_iterator will just stop at the first non-convertible element (std::stod throws std::invalid_argument).

Also adding some cosmetic changes which were lost in the review of https://github.com/electronic-structure/SIRIUS/pull/980

simonpintarelli commented 2 months ago

cscs-ci run default

simonpintarelli commented 2 months ago

cscs-ci run default

toxa81 commented 2 months ago

@abussy I changed a bit logic for is_upf_file(). Can you please check if this version or my version is better?

abussy commented 2 months ago

Where can I find your version @toxa81 ?

toxa81 commented 2 months ago

Where can I find your version @toxa81 ?

In the merge conflict src/unit_cell/atom_type.cpp

abussy commented 2 months ago

@abussy I changed a bit logic for is_upf_file(). Can you please check if this version or my version is better?

I have no strong opinion on this tbh, both implementations seem functional to me

simonpintarelli commented 2 months ago

@toxa81 My version avoided to check the entire string (which might be an entire UPF file of several MB). Seems better to me to only check the last 4 characters.

toxa81 commented 2 months ago

@toxa81 My version avoided to check the entire string (which might be an entire UPF file of several MB). Seems better to me to only check the last 4 characters.

It was breaking for zero size input string. I didn't think of a large byte string that is not a file name.

toxa81 commented 2 months ago

@simonpintarelli I changed is_upf_file().. hopefully for better :)