Skip to content

Conversation

@newville
Copy link
Member

@newville newville commented May 18, 2025

This adds FDMNES outputs to the larixite web/Flask app (addressing #6).
It also adds a Zip file to download all outputs, including the CIF file.

This also adds a couple of convenience functions for struct and fdmnes to allow generating structure results not from a filename/Path, but from the text of a CIF file, and not writing files but returning the text of the output files. That is, without doing I/O in the calculation function (Separating calculation from I/O is sort of like the MVC discussions -- generally a good idea, sometimes hard to always follow in practice).

Unfortunately, pymatgen's XYZ function needs a real file or Path, and cannot use a StringIO(text) buffer. If that gets resolved, then get_structure could call get_structure_from_text, but at the moment that is sort of clunky, so I ended up not doing that, and leaving sort of duplicate code that should be cleaned up (like, when pymatgen.io.xys.XYZ can read from StringIO).

Anyway, this branch works for me for local generation of FDMNES files and Zip Files of all outputs.

@maurov
Copy link
Member

maurov commented May 20, 2025

@newville it looks great to me! Thanks for implementing this so quickly. I did only a quick look, but I am tempted to merge it and release 2025.6.0 right now. What do you think?

I have only observed that the Larixite version does not get exported correctly in the FDMNES input file.

Furthermore, it would be great to follow good practices and add some tests for this, if possible

@maurov maurov added this to the 2025.6.0 milestone May 20, 2025
@newville
Copy link
Member Author

I would like to try this on a production web app before merging. I think it should all be OK, but adding generared zipfiles probably needs testing on a real server. Hopefully today!

@maurov
Copy link
Member

maurov commented May 20, 2025

OK, I agree. There is no hurry from my side. This can wait.

In the meantime, I will go deeper in the code, especially for the part separating the calculation from I/O. I think I will propose few changes there.

@newville
Copy link
Member Author

FWIW, the web app at https://millenia.cars.aps.anl.gov/larixite now redirects to https://larixite.seescience.org/ (which is the same server) and is using this branch.

So, I think this is working well enough to release.

@maurov
Copy link
Member

maurov commented Jul 23, 2025

@newville I have lost track of this pull request. Should we simply merge it?

@newville
Copy link
Member Author

@maurov yes, and thanks for the reminder. The web app is working to generate zip files of FDMNES files, so it should be fine to merge this.

@maurov maurov merged commit 7f72ad7 into main Jul 23, 2025
4 checks passed
@maurov maurov deleted the webapp_mods branch July 23, 2025 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants