Skip to content

Scripts for form-factor dataset creation - #486

Draft
comcon1 wants to merge 3 commits into
NMRLipids:mainfrom
comcon1:form-factor-dataset
Draft

Scripts for form-factor dataset creation#486
comcon1 wants to merge 3 commits into
NMRLipids:mainfrom
comcon1:form-factor-dataset

Conversation

@comcon1

@comcon1 comcon1 commented Apr 8, 2026

Copy link
Copy Markdown
Member
  • script for recomputation of dataset with fragmentized densities
  • script for CI storing of Databank FF/density info into an easy-digestable format

📚 Documentation preview 📚: https://databank--486.org.readthedocs.build/

@comcon1 comcon1 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some pack of comments

Comment thread developer/make_train_data.py Outdated
Comment thread developer/make_train_data.py Outdated
Comment thread developer/make_train_data.py Outdated
Comment thread developer/make_train_data.py Outdated
removed commented code, dev prints and provisionary get_mean_ApL
function
@comcon1

comcon1 commented Apr 11, 2026

Copy link
Copy Markdown
Member Author

@BananaOverLord I've cleaned a bit. Please see the comments. The easiest way to run test sweep:

cd ~/workdir
fmdl_initialize_databank toy
source databank_env.rc
/path/to/repo/develop/make_train_data.py

It does some trajs but crashes at some point. You can debug further.

Please add Ruff extension to your vscode and then add to your project/.vscode/settings.json

    "ruff.configuration": "developer/ruff-dev.toml",

This is the list of ruff's recomendations we are trying to obey. Your code is quite clean so I didn't do much. And it's semiautomatical with ruff. If you don't use vscode, you can add ruff to your IDE. And also periodically do tox -e lint tox -e format before committing.

u.load_new(traj_centered, format="XTC")
u.guess_TopologyAttrs(force_guess=["elements"])
mollib.guess_elements(system, u)
return u

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reshaped this function a bit. Now it returns the Universe.

Comment on lines +321 to +328
def recompute_extended_ff_dataset(
h5fpath: str, *,
hydration_threshold: int = 20,
recompute_centering: bool = True,
small_trajs_only: bool = False,
) -> None:
"""
Recompute the extended form factor dataset for all systems in the databank.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrapped into a function. I prefer having just one line in main. Then we can call it as a module func.

scalar_info = {"ApL": ApL, "thickness": thickness}

water_to_lipid_ratio = water_n / lipid_n
if system.get_hydration(basis="number") < hydration_threshold:

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's important. It's our API. It allows to remove a whell-invention block above.

)

molecule_types_selector = [
f"resname {system['COMPOSITION'][molkey]['NAME']}" for molkey in system.content

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's important to know.

for molkey in system.conent:
   # will go over all molecule keys

@comcon1 comcon1 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit of comments to my cleanings.

@comcon1

comcon1 commented Apr 11, 2026

Copy link
Copy Markdown
Member Author

Substantional questions:

  • are you sure that you want number hydration and not mass hydration for your threshold? we have cardiolipins - they are two times bigger. We have also small molecules like beta-OG or C20.
  • we cannot select molecules using NAME: key as a resname selector. You have amber which separates molecules. If you want to select molecules in amber, you must use MDAnalysis mechanism and then define which molecule is it. It's not trivial. If you don't want to do this stuff - you MUST check that mapping contain ONE residue per molecule. And exclude all other
  • I don't understand what you are going to do with molecules that don't have standard structure of two-tailed phospholipids. For example, what are you going to do with sphingomyelines? With cardiolipins? With GB3? With BOG? With alkanes? With other synthetic surfactants? This fragmentation story should be thinked and developed a bit.
  • I don't see how you are going to make this script actually running. Are you going to never restart it? It doesn't have any mechanism to compute only what have not been computed. Our recalculation engine has this mechanism, but you didn't want to use it (maybe it's better for this HDF5). It's impossible to recalculate everything without being stuck at some point.

@BananaOverLord

Copy link
Copy Markdown
Collaborator

To your last comment with substantial I've talked with Hanne and Samuli.
-yea we can use mass hydration. Do we have the function in the repo?
-I'll look into it
-we will leave it at that. What is set as "tail" (as it should be in the membrane) or we can remove these membranes with weird stuff on them (I think they are in small minority)
-I'll make it append by default and add flag to recalculate everything. Should I add some checking mechanism to control if the membrane has been changed to recalculate?

@comcon1

comcon1 commented May 14, 2026

Copy link
Copy Markdown
Member Author

-yea we can use mass hydration. Do we have the function in the repo?
Yes, we have. system.get_hydration(basis="mass") https://nmrlipids.github.io/FAIRMD_lipids/latest/auto_gen/fairmd.lipids.core.html
-I'll look into it
-we will leave it at that. What is set as "tail" (as it should be in the membrane) or we can remove these membranes with weird stuff on them (I think they are in small minority)
ok.
-I'll make it append by default and add flag to recalculate everything. Should I add some checking mechanism to control if the membrane has been changed to recalculate?
I answered in the letter.

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.

2 participants