Skip to content

Comments

Factorize Models to Prepare HF Upload#34

Merged
LTMeyer merged 5 commits intomasterfrom
factorize-models
Mar 25, 2025
Merged

Factorize Models to Prepare HF Upload#34
LTMeyer merged 5 commits intomasterfrom
factorize-models

Conversation

@LTMeyer
Copy link
Collaborator

@LTMeyer LTMeyer commented Mar 25, 2025

Context

We want to upload the checkpoints of the model trained for the benchmark to HF (see #33).
To ease the loading of the checkpoints from_pretrained(<hf_url>) and not having to pass any redundant parameters this requires all the parameters of the model class to be of standard type. This is not the case as we use the dataset.metadata for instantiation. This PR fixes it by setting explicitly the n_spatial_dims and spatial_resolution arguments that were originally extracted from the metadata.

Content

  • Add a BaseModel class to support HF upload;
  • Change dset_metadata -> actual n_spatail_dims and spatial_resolution arguments:
  • Add tests for most of the models.

Copy link
Contributor

@francois-rozet francois-rozet left a comment

Choose a reason for hiding this comment

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

The approach seems sound to me, if the training scripts still run. I just left a minor comment.

Co-authored-by: François Rozet <francois.rozet@outlook.com>
@LTMeyer
Copy link
Collaborator Author

LTMeyer commented Mar 25, 2025

The approach seems sound to me, if the training scripts still run.

I've just tried the following and it worked as expected.
python train.py experiment=fno server=rusty data=active_matter trainer=debug

@rubenohana
Copy link
Contributor

To be sure, does it work for 3D data as well?

Copy link
Contributor

@mikemccabe210 mikemccabe210 left a comment

Choose a reason for hiding this comment

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

Looks like a solid improvement to me, especially the test additions!

@LTMeyer
Copy link
Collaborator Author

LTMeyer commented Mar 25, 2025

To be sure, does it work for 3D data as well?

It should. If you have a command you would like me to test let me know.

@rubenohana
Copy link
Contributor

To be sure, does it work for 3D data as well?

It should. If you have a command you would like me to test let me know.

Could you try the same command for training on the MHD_64 dataset?

@LTMeyer
Copy link
Collaborator Author

LTMeyer commented Mar 25, 2025

Looks like a solid improvement to me, especially the test additions!

With the tests I've noticed by the way that some models seem to accept B H W C while other expect B C H W as input shapes (see https://github.com/PolymathicAI/the_well/pull/34/files#diff-ca99d4b52e8a6a4e8fdadd6871c1152341a654ab0c18f503fbdb715820cad05fR58 and https://github.com/PolymathicAI/the_well/pull/34/files#diff-e6fd5adf396bade4ea5c4661206b237722692d3f31beb865ed0720ea149a76e4R17-R21). I don't remember if this was expected?

@LTMeyer
Copy link
Collaborator Author

LTMeyer commented Mar 25, 2025

To be sure, does it work for 3D data as well?

It should. If you have a command you would like me to test let me know.

Could you try the same command for training on the MHD_64 dataset?

I've just checked the following command runs as expected python the_well/benchmark/train.py experiment=fno server=rusty data=MHD_64 trainer=debug.

@LTMeyer LTMeyer merged commit dc7b10b into master Mar 25, 2025
4 checks passed
@LTMeyer LTMeyer deleted the factorize-models branch March 25, 2025 17:09
LTMeyer pushed a commit that referenced this pull request Apr 4, 2025
…-computation-of-statistics

33 issue with dataloader and computation of statistics
LTMeyer pushed a commit that referenced this pull request Apr 4, 2025
…-computation-of-statistics

33 issue with dataloader and computation of statistics
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.

4 participants