Bug report: Application crashed with ValueError when setting a custom UPF file

[giovannipizzi]

Automated report

This bug happens when I upload a custom UPF.

Expected behavior (here I have only one chemical element, gold, to discuss if what I write should be the same in general for multiple elements).

1. if I customize the pseudos, I should have a fifth option on the top, or a checkbox that shows the custom UPF section
2. When I upload a file ,the counter should not continue to increase at every upload
3. Not sure if the text box is useful? First, the filename does not change, second probably I would just show a label with the updated filename? Or, if it's a text box, it should reflect the filename uploaded, and could be renamed by the user if they want
4. The app shouldn't crash :-)

Additional comments (optional):

This is by following the basic in-app tutorial for Gold and trying to change pseudo to a file uploaded.

Attachments

Traceback

/opt/conda/lib/python3.9/site-packages/traitlets/traitlets.py in notify_change(self, change)
   1515     def notify_change(self, change):
   1516         """Notify observers of a change event"""
-> 1517         return self._notify_observers(change)
   1518 
   1519     def _notify_observers(self, event):

/opt/conda/lib/python3.9/site-packages/traitlets/traitlets.py in _notify_observers(self, event)
   1562                 c = getattr(self, c.name)
   1563 
-> 1564             c(event)
   1565 
   1566     def _add_notifiers(self, handler, name, type):

/opt/conda/lib/python3.9/site-packages/aiidalab_qe/app/configuration/advanced/pseudos/pseudos.py in _on_file_upload(self, change)
    378         # the pseudo_filename is set
    379         with self.hold_trait_notifications():
--> 380             self.pseudo = UpfData(io.BytesIO(content), filename=filename)
    381             self.pseudo.store()
    382 

/opt/conda/lib/python3.9/site-packages/aiida/orm/nodes/data/singlefile.py in __init__(self, file, filename, **kwargs)
     52 
     53         if file is not None:
---> 54             self.set_file(file, filename=filename)
     55 
     56     @property

/opt/conda/lib/python3.9/site-packages/aiida_pseudo/data/pseudo/upf.py in set_file(self, source, filename, **kwargs)
     86         source.seek(0)
     87         content = source.read().decode('utf-8')
---> 88         self.element = parse_element(content)
     89         self.z_valence = parse_z_valence(content)
     90 

/opt/conda/lib/python3.9/site-packages/aiida_pseudo/data/pseudo/upf.py in parse_element(content)
     28             return match.group('element')
     29 
---> 30     raise ValueError(f'could not parse the element from the UPF content: {content}')
     31 
     32 

ValueError: could not parse the element from the UPF content: Not Found

Environment fingerprint

eNp1WE2T47YR_StTytVUiaSkWe3N2ZvLm2wy67hSqZQKJCEKIxDAAKBmOC7_9zS6GxLG5Rz5AOKj-3X3a_y2ukoflDWrz_UPK6dF
PFk_rT7_thK-P6so-zh7ufr8n9V-26m4-mG1-i_MW-LZmuPt11W7PqzrFkbv0F8ObfVLN5s4Pzx9_fbwJOLDV-
Ef6t1Dvf1cN5-bw8Mv3788NJtmt_odlhT9RYwy4NbD8xzid_kWYZ163a43sLJQahCViHZSIaoeRjbrer0R97FODNIzvr-hvU3nXz
Xr_bq9g665IFiv6xs4ijkEJUw1iCji4tJhYLFdMeUMqH-Z5SxxqClOpiY3exWXMMOUXmrdW3PF82-KBaYZjZPA7Q10YEzrFsTLJV
2Q82ARfixhtP6z7fgM94VeZgH2nmRwXoaQ_tyu603xazjDwfC3TwV6VZ23of-zI7wKY5T0hw1a609HqlfrLydtXwPOKb2VRkYv3P
mPhtQiHb7ZAm02BXY82wmdtSu3SgMvDG8_rFG9yNKifxz6__dKS76qYZQxHDsRiCBbHreVUxcB0B6sRGtaiAQfrdVEiTovBbHygv
RvaaKWU4fcrPOvxlggkxyqO6HYlyao5hwnjbMP6waxRVlcb89_L3RK_I9DTDg3KB_wty0537nJDrL0awreq6yciGT7La3vR2uaqj
-dVLrxLbTucNUpMygzBhrne6KB2nXDFgox2os05G_YErHF9MdRGukhRD0ersbJMeJZmxRrCfir6KSm2NtgRHYQ-b1gWtKGCXLWx7
A-zaZHu1faz1Uv-rPEtfd48a73i4tI8wYN1kkxR3WadbCz2xL_KcQ7LeFnQHZMlE4rc5F00l36hiudmQd4LBttS_nncd18YoSzSQ
F6G7VChmXPfkmn_GJN9JYuVRM78PSZRFsO3B6sKZ0ybo6cutBsBL-BNxAlP_U3Y8KC6NAeWKnImZBHAWzrjPKRyC7scVh8x9_jFV
1KHu3PwgcZKwN8Flq9o1lycuyV72fmPbGr16pP6RMojs7H7yrALcwtAdMttJ0HB4OaCMQLWm29mAQTc5ehicgKd6VdrBkEMedwBy
ouFNVZmAFcOHL48IRoZ8_hzjaDTF-heat7_B7oHGnIzvHj2I5-S8SymLvSatsts6Zfep3vSKYdZDePvCWVmEECSzgKdnznQZ7mII
e3STNRCIQ8DU6lgpkDZFAcC1BU8TNcjrZ7hkocYmZfwzMDcK8rJ5tA1QHXIzoOQFpH5NojaQAI0YPpji65nW-zxxEnB46Bx_Q961
fVUwYBNyRIQqTP5mT1wNsmAkjIe9JEzjZbTkJyEkofr0Coga2RK4icJmAbxxIkECIBrABWt8pE8kQKDzmn4sgplTwq33rpks1GD0
HO5mhwQPYQ_UwJsvsJ4ug5WBMg9JBxYGWywklpqS3yuM3EPmkVj3yslp10AlLdg3ZHNj3NJIuw7CQAcnTFuR5OxtGC-yCHruqdjE
gpb_RSGi0jHzRB5x1yqE3JIi14Vi5lcMry6XuehDl5JYH1SDbaVw1gNXVaWOEcEDKCYjt9TBArWFOalLLwlymlVuBNBU4QSetgqc
sL3kZBQkAw9Vi0drdht0DSNJjB98mUDALDYKuhlEXqxsTHdbstoQrqBDiKA-4-G6zCSEMIl2hc4pHucwMrkIdgDORr9raKYRBmlE
CMUNDtWQ4q56807SdlnkXDGSlt9ZwkE5E36wiQVxRaNdfOxKIdZ48df0P-kcQLss4Hpm2zeZ5nuJv0R0iT4CyyBy2RB4hxt1rFeA
WxeeUKVR8-jESMOAiXLIPvY-e5K5Iog6S3Urr_9AEELTmmyA2F3e-jx9sBmlQ4_jB4dw6aPx3vAgJIoWLNheeiXu_F8ZGBYHW-F-
UZLfylCGToQiDIQdH0xU20eF-O2t40fmIDeGgSUyc4_VJt0_o6QRwTlbYUTRAFIxecPdWKr-JiecsWJ8AyxSLQqlwG-0q9zSMDs3
sSJ1mURqhYTlvOwakH2n4Aoe4khhQtSTGmkw6qMoOycEvNzWwkKytcDhKdnShRbTkFAhaXXPRbugBwqCpVap3SPU52lXDE_-2O5j
rwuz_bogBPbsr85y3CmCotexeRWQZfJLZp1idqw_igs45qUH2k1ID_mK7XApqqvtgqYRwIWTEBBM0SnB2Dg5hjuhNKAsw-JIfBVJ
Blk9JUln21RzimRuONNCrNHPVVyVemT-K8ATod770IXOHAaIVoVQhtuk8ak9zCUfRBepedtRe8IERrU2DQWqmpiEYzTw2carj1kF
sCO2bhJs_KzQkpWgv69Zz5RHcpUmtKTunG1qdMQwmMSzfJIip-DZUUB9EYo-hYfLUMTlKLonIB8OKKzs2BrhI5e7YM2B7KJZArsN
lpIog4daHawh1REhSW6Yz_yjcn-1iIAjeGuehb760vbMfrol4MIEklU2SHqIYWM22f-fhNMYca3Ek56hApfbvLmCrMMRUxSDZHIy
bW7aRM80sHd1IpsNCKKTgXpFxDrabT7CLYp8ZM4DQXahz2EtL3NKncheDiHtrYeJZzqAqq17wgDDoSnBf8KzG05RGXO5ycP11I18
ADUYvowgJybmzQYgcs-BlKvZvwy8eR0Qhd9qwO-lBvoaqXvagDMVPJq9BFGXBLagXhkuheNs_SQWamSVTRvy1f1Omfv-Lltzynt0
FkvdkSchOasHZN0Ow1RvuWEq5boHLTuw52hvsCO97ecJpHPhsNQIWMMXer3HGnsXDG7mRDzFlAfBMJU-3e46lvVa_Jguvb8tOv3x
FgGi6QfECoUJrdphRUwqD2hF6CChWtHvI8UI0HmpcSdy7rLe7wdXn6x898DkL-Jr7oMqaWvztpnp5-zlwmMJmPQjuRhZYHBFrEzC
26-7flCQpF4BcjsgYKLlB5komU7NQUI5C_OMnVvBviKcFU2o4ju22Tl_NiYeW_JeCdbp7v-O8fv_6MCXLD27zjE0mTO6gXeWu2cs
p5gaM5bs3Tol6OyDJcteZe0ssXKEDss5ZSU8aKhwEKGz-nNLdeBPdb9SPxrsDXfW6dGuQEDVVpqMqt5ybJnk9p99DCxU045W6JRH
noFcRwhSK7jArGtRTecGNYE8zZnl8EghSd9SY_EjQIyctNih4w4QQ5kVxq2T5PoHub755oXnNiDakRAGEeitwO4TG7bO19fj2D1H
qRpvNSXLgw7BEF3T_PasgiOBk4qDc6L0mJMAkfK2gSSdUTYYJRpxMXZPJnenwJSl4l-7hZOxsibu1GsnqOMoiHH3VSzAuzrE3FOo
QzqCDqMjb8RhEiVLiK25X8mBiW6ZR7wy193y2cLoDl7_ihtjM8J5XJsZjWhzAQvUJZdWCFAwVvwpcJtn3NGKRZFrlsl3gGYw4ODN
1Hze93iINlUp-ZcwMURYLN0ofQFJ1LtJmpFDbpWxWBl5z4XuZxaKj5FNTOxpdhwmSzJycDXZUJMT8u1DcwahLtu3zvxGt171qpzE
CVSA8E2GMFHkuKhwbf2Q2oPxMysx659YmzSY9QcISoUN-2XFVmM6jbcWZ3OkZ75H9zYwWF4f4QxU1CSzhwp81qCcvvvwR0q0shSq
8KSAy6B1Na6rbpOFcVZPHyc7Ua4mK2-VmLLvEqYn8e7MgNXZr42gN1-Am1wdO9ShFKPsG2BHfSQMvCxSjzE-
AAGTk9dmQhUHMUvp4l87ulVE2LHp30yiYZfRNudTEcTHfzCRMNjw4Sll8TqEK-Tbp48qDDLMJTralJnL0r57hv2ax-
__1_EENfIg==

By submitting this issue I confirm that I am aware that this information can
potentially be used to determine what kind of calculation was performed at the
time of error.

[edan-bainglass]

@giovannipizzi question - are we suggesting the user to change the pseudo to a custom one, or are you just testing capabilities?

[giovannipizzi]

No, it's not for the tutorial, it was just to test something on pdos that a participant asked me, and I need to use a different pseudo that is not among the 4 we provide. So not urgent but would be nice to get this fixed eventually

[edan-bainglass]

After some discussion, we conclude that this needs more discussion. Will mention in next meeting.

[edan-bainglass]

@giovannipizzi see below the code for what happens when a user uploads a file

    def _on_file_upload(self, change=None):
        """When file upload button is pressed."""
        filename, item = next(iter(change["new"].items()))
        content = item["content"]

        # Order matters make sure when pseudo change
        # the pseudo_filename is set
        with self.hold_trait_notifications():
            self.pseudo = UpfData(io.BytesIO(content), filename=filename)
            self.pseudo.store()

            # check if element is matched with the pseudo
            element = "".join([i for i in self.kind_name if not i.isdigit()])
            if element != self.pseudo.element:
                self.error_message = f"""<div class="alert alert-danger"> ERROR: Element {self.kind_name} is not matched with the pseudo {self.pseudo.element}</div>"""
                self._reset()
            else:
                self.pseudo_text.value = filename

I modified it to the following

    def _on_file_upload(self, change=None):
        """When file upload button is pressed."""
        filename, item = next(iter(change["new"].items()))

        if (
            orm.QueryBuilder()
            .append(
                UpfData,
                filters={"attributes.filename": filename},
            )
            .count()
        ):
            self._reset_file_upload_widget()
            self.message_box.message = f"""<div class="alert alert-warning"> Pseudo with name {filename} already exists in the database.</div>"""
            return

        content = item["content"]

        # Order matters make sure when pseudo change
        # the pseudo_filename is set
        with self.hold_trait_notifications():
            self.pseudo = UpfData(io.BytesIO(content), filename=filename)

            # check if element is matched with the pseudo
            element = "".join([i for i in self.kind_name if not i.isdigit()])
            if element != self.pseudo.element:
                self.message_box.message = f"""<div class="alert alert-danger"> ERROR: Element {self.kind_name} is not matched with the pseudo {self.pseudo.element}</div>"""
                self._reset()
            else:
                self.pseudo.store()
                self.pseudo_text.value = filename

            self._reset_file_upload_widget()

    def _reset_file_upload_widget(self):
        self.file_upload.metadata = []
        self.file_upload.data = []
        self.file_upload._counter = 0

Comments:

1. _reset_file_upload_widget effectively ignores the counter, which in principle we don't need and is unfortunately not simple to remove
2. UpfData(io.BytesIO(content), filename=filename) handles parsing. It is a Data type provided in aiida-pseudo that handles the parsing

[edan-bainglass]

From a recent email exchange:

@giovannipizzi's ideal situation below (plus @edan-bainglass's replies):

PBE or PBEsol always selected

When a pseudo is selected, we check if it matches, if not, we show a red warning on the right

By selected, I assume you mean uploaded. Also, matches what? The functional? Otherwise we show a warning?

So far in #1348, I've restored the warning system. If the filename of the uploaded pseudo exists in the DB, I warn the user and ignore it. Otherwise, the uploaded pseudo is parsed. If it is of a different element, I block it and show a message. If it matches the element, it passes, is stored and assigned to the pseudo dictionary input for the workflow (by UUID). One issue I should check is if they load something that isn't a UPF (wrong format). Should catch this in a try/except.

If a custom pseudo is selected, none of the 4 pseudo library buttons is selected, And a sentence says there is at least 1 custom pseudo, and says that to reset, they have to click on one of the 4 buttons - in which case, cutoffs are reset and custom pseudos removed.

Sure, that's one of the options I provided in the Figma (or maybe a bit of a hybrid). This I can do.

Also if the pseudos are the same but cutoffs are changed, a warning is mentioned below the 4 buttons (in this case the button can remain selected, probably we need a reset button in that sentence).

Unclear what you mean by "pseudos are the same". The uploaded pseudo and the currently selected one?

The pseudo selection should clearly show if a custom pseudo has been selected (or not), and allow to download it back

If we want a download feature for pseudos, this should be done in general. Maybe you can clarify the use case?

It also parses it and shows the functional (and check the chemical element) (and any simple information we can show if possible, e.g. if it’s ultra soft etc.)

As mentioned, if the uploaded pseudo passes the first check (existence by filename - might not be the most robust), it is parsed and we have access to whatever is parsed by the UpfData class. We can consider showing details for it, but then why not for all pseudos?

The cutoff would show the recommended cutoff if we have that pseudo in our hardcoded tables (and saying “recommended cutoff from SSSP tests” or “recommended cutoff by PseudoDojo", otherwise the cutoff sentence on the right is hidden

More discussion on this is needed. I'm not familiar with the tables, but I assume that if they exist, they are remote, so we need to make an API call for this? Unless this is info codified in the Family nodes.

I don’t think then we need a checkbox (we can just mention, if a pseudo is changed: to reset the pseudos, go back into the main panel and pick a family)

Main panel? When did we switch panels? 😅

Either selecting a family already sets all pseudos and cutoffs correctly (preferred), or if they have a ’none’ state, as soon as one is changed, all pseudos should be set to those recommended by the previous pseudo family choice, before deselecting the family buttons.

Currently, selecting a library or functional recomputes all pseudos given the selected protocol. So doing so after having selected a custom pseudo will overwrite with defaults (though the custom pseudo will still exist in the DB). This is mostly option 1 in the Figma. You deselect all on at least one custom PP upload. Reselecting anything will recompute defaults w.r.t protocol.

[edan-bainglass]

One note is that the functional is not parsed at the moment.

Update

upf-tools does the trick - thanks @mbercx for the suggestion 🙏

[edan-bainglass]

@giovannipizzi this issue is addressed in #1348, which is now merged. We'll release it to the demo server shortly. Good if you can test it once it's up, to see if this issue can be considered closed.

Thanks to @mbercx for the required changes in the plugin (and @t-reents for subsequent reviews) 🙏
Thanks to @superstar54 and @AndresOrtegaGuerrero for testing and feedback 🙏