Abinit phonon workflow#1
Conversation
| if True avoids the creation of inputs that will produce duplicated outputs. | ||
| qpt_list: list or tuple or None | ||
| A list of q points to compute the phonon band structure. | ||
| All the q points must be part of the k-mesh used for electrons. |
There was a problem hiding this comment.
Questo non è proprio formulato correttamente. In teoria il punto è che le griglie siano commensurabili. Tecnicamente perché quello che deve essere è che k+q appartenga alla griglia k per tutti i k e q. Quindi se hai una griglia NxNxN di punti k ma shiftata, una griglia di punti q NxNxN è ok, ma non è vero che i punti q appartengono alla griglia dei punti k.
| ddk_maker: BaseAbinitMaker | None = field(default_factory=DdkMaker) # | | ||
| dde_maker: BaseAbinitMaker | None = field(default_factory=DdeMaker) # | | ||
| dte_maker: BaseAbinitMaker | None = field(default_factory=DteMaker) # | | ||
| wfq_maker: BaseAbinitMaker | None = None # | not implemented |
There was a problem hiding this comment.
Per quello che mi riguarda se il valore è definito deve essere evidente che non può essere usato. Da qualche parte deve esserci un raise che avvisa che la funzionalità non è implementata se questo valore non è None quando l'oggetto viene creato.
il __post_init__ dovrebbe essere un punto adeguato.
| self, | ||
| structure: Structure | None = None, | ||
| restart_from: str | Path | None = None, | ||
| **anaddb_kwargs, |
There was a problem hiding this comment.
Non credo che questo sia il posto giusto per questo. Tutti gli input per gli eseguibili vengono settati tramite l'InputGenerator o come argomenti del Maker associato. Mettere qui gli argomenti di anaddb sarebbe una rottura con lo schema standard.
| use_dde_sym: bool = True | ||
| dte_skip_permutations: bool | None = False | ||
| qpt_list: list[list] | None = None | ||
| ngqpt: list | None = None |
There was a problem hiding this comment.
immagino che qpt_list e ngqpt siano alternativi e non possano essere definiti contemporaneamente? Oppure Sì? Se no, mettere qualcosa nel __post_init__ che verifiche che solo uno sia non None
|
|
||
| static_job = self.static_maker.validate_grids( | ||
| structure, ngqpt=self.ngqpt, restart_from=restart_from | ||
| ) |
There was a problem hiding this comment.
Onestamente questo approccio non mi piace molto per una serie di motivi:
- tecnicamente il punto di poter passare un maker arbitrario a
static_makervincola questo check. È vero che non è un caso molto improbabile, ma è un vincolo che non è strettamente ragione d'essere - Introduce un nuovo metodo che genera
Job. Non credo che sia una buona idea, perché confonde sull'utilizzo del Maker. Questo Maker genera due tipi di Job. - un metodo che si chiama
validate_gridsnon mi aspetto in realtà che poi ritorni il un job che faccia lostatic_job.
Personalmente farei così:
- il check dovrebbe essere indipendente dal tipo di StaticMaker
- Deve essere necessariamente un Job? Non può essere fatto direttamente qui? (in realtà credo di no, ma chiedo per sicurezza)
- qui sta valutando solo
ngqpt. Eqpt_list? viene validato da un'altra parte? Oppure manca?
Se deve essere un Job a parte, sinceramente mi chiedo se il gioco valga la candela. Forse si potrebbe fare il check in generate_phonon_perts e basta. Se bisogna fare dei check complicati allora tanto vale investire il tempo e implementare i wfq, così non c'è bisogno del check.
| born = None | ||
| supercell_matrix = None | ||
| primitive_matrix = None | ||
| epsilon_static = None # ??? |
There was a problem hiding this comment.
questo potrebbe essere in realtà quello che qui è chiamato epsinf o forse invece è diverso. Ci sono due valori che si possono estrarre (vedi per esempio: https://www.nature.com/articles/sdata201865/tables/6).
Per il workflow di phonopy dovrebbe essere questo: https://github.com/materialsproject/pymatgen/blob/5df11d9acb031741a5cc5f03b56b76de21a1ed23/src/pymatgen/io/vasp/outputs.py#L576
bisogna controllare a cosa corrisponde esattamente
| """Create an AbinitStoredFile from the original file.""" | ||
| source_filepath = os.path.abspath(filepath) | ||
| if data_type in {"bytes", bytes}: | ||
| if data_type in {"bytes", bytes} or ".nc" in str(filepath): |
There was a problem hiding this comment.
perché questa aggiunta? ci sono casi in cui non viene passato bytes come data_type? Altrimenti eliminerei questo check aggiuntivo, visto che data_type è un argomento obbligatorio
| def load_manager(abinit_test_dir): | ||
| import abipy.flowtk.tasks | ||
|
|
||
| abipy.flowtk.tasks.TaskManager.USER_CONFIG_DIR = str(abinit_test_dir / "abipy") |
There was a problem hiding this comment.
per mia conoscenza, come mai questa aggiunta? è cambiato qualcosa in abipy?
|
|
||
| @pytest.mark.skipif( | ||
| which("abinit") is None, reason="abinit must be installed to run this test." | ||
| ) |
There was a problem hiding this comment.
abinit non deve essere necessario per girare i test. Tutti gli oputput sono fatti con un mock, abinit non deve girare. Perché questa aggiunta?
| assert user == ref | ||
| ref_abivars = ref.structure.to_abivars() | ||
| # Check structure | ||
| for k, user_v in user_abivars.items(): |
There was a problem hiding this comment.
come mai questa implementazione aggiuntiva dei check? In teoria pytest dovrebbe comunque mostrarti gli oggetti che falliscono un confronto quando hai qualcosa del tipo assert a == b
2 participants
Summary
Ho modificato il flow:
- DfptMaker per permettere il calcolo dei fononi
(ho scelto quel modo di passare gli kwargs a anaddb che so non essere ottimale perche' abipy.abio.inputs.phbands_and_dos ha bisogno di ngqpt come input e se l'utente non lo passa come input lo devo recuperare dal job per la creazione delle perturbazioni dei fononi, fammi sapere se pensi ci sia una soluzione migliore)
Nei jobs/core:
- ho aggiunto un metodo a StaticMaker per validare le griglie e vedere se restituire il maker.make o ValueError
Nei jobs/response:
-oltre al PhononResponseMaker, ho aggiunto un job per creare la lista di punti q dalla griglia ngqpt e calcolare le corrispondenti perturbazioni indipendenti simmetricamente. Qui restituisco dagli output anche la griglia ngqpt per i motivi della parentesi sopra.
per la parte ANADDB TaskDoc ho provato a riprodurre la classe common.schemas.phonons.PhononBSDOSDoc, ho fatto al meglio che potevo utilizzando l'interfaccia da abipy a phonopy, alcune cose credo che semplicemente non siano applicabili, pero' il resto dovrebbe funzionare
per la questione mrgdv, come avevamo discusso ho fatto un po' di copia e incolla da come e' implementato mrgddb e dal wrapper AbiPy, poi per non lasciare l'output del job completamente vuoto, ho notato che nel log vengono date alcune informazioni su come sono stati calcolati i fononi e quindi le ho aggiunte all'output, non so se sia una cosa utile, le caso si puo' togliere.
Additional dependencies introduced (if any)
Ho poi aggiunto ad AbiPy una factory phononpert_from_gsinput() e una metodo di Abinit input che ricalca il phonon_from_gsinput ma restituisce, dato un punto e la perturbazione, l'input per la singola perturbazione, anche qua come avevamo discusso.