SIRIUS icon indicating copy to clipboard operation
SIRIUS copied to clipboard

add size check / simplify vec_from_str

Open simonpintarelli opened this issue 10 months ago • 9 comments

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

  • is_upf_file, only check last 4 characters (not the entire upf file passed as string)
  • static_cast instead of c-style cast
  • add sanity check for d_ion matrix size

simonpintarelli avatar Apr 16 '24 11:04 simonpintarelli

cscs-ci run default

simonpintarelli avatar Apr 19 '24 09:04 simonpintarelli

cscs-ci run default

simonpintarelli avatar Apr 19 '24 10:04 simonpintarelli

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

toxa81 avatar Apr 30 '24 07:04 toxa81

Where can I find your version @toxa81 ?

abussy avatar Apr 30 '24 08:04 abussy

Where can I find your version @toxa81 ?

In the merge conflict src/unit_cell/atom_type.cpp

toxa81 avatar Apr 30 '24 09:04 toxa81

@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

abussy avatar Apr 30 '24 09:04 abussy

@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.

simonpintarelli avatar Apr 30 '24 09:04 simonpintarelli

@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 avatar Apr 30 '24 09:04 toxa81

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

toxa81 avatar May 07 '24 16:05 toxa81