Model names

[mastoffel]

• changes self.models to dict with model_names : model
• changes get_model_name() to get model name from that dict
• having two pytorch models with the same class works now

resolves #175 and #190

[github-actions]

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
autoemulate
compare.py					243
cross_validate.py					57
hyperparam_searching.py
model_processing.py
printing.py
save.py					56
utils.py					91, 97
autoemulate/emulators
gaussian_process.py
gaussian_process_mogp.py					71-83, 87, 90
gradient_boosting.py
neural_net_sk.py
neural_net_torch.py
polynomials.py
random_forest.py
rbf.py
support_vector_machines.py
xgboost.py
autoemulate/emulators/neural_networks
rbf.py
tests
test_cross_validate.py
test_emulators.py
test_model_processing.py
test_printing.py
test_save.py
test_torch.py
test_utils.py
Project Total

The report is truncated to 25 files out of 29. To see the full report, please visit the workflow summary page.

This report was generated by python-coverage-comment-action

[bryanlimy]

LGTM.

Have one suggestion for NeuralNetTorch model names.

Also, do you think it would be useful to have a getter function / attribute for all emulators that return the model name? e.g.

> model = NeuralNetTorch(module='mlp')
> model.model_name
NNMLP
> model = RandomForest()
> model.model_name 
RandomForest

[bryanlimy]

Should we have a prefix of NN followed by <module name> for all NeuralNetTorch model?

e.g.

• "NNMLP": NeuralNetTorch(module="mlp")
• "NNRBF": NeuralNetTorch(module="rbf")

so that

• users can immediately tell they belongs to NeuralNetTorch
• they will be close to each other when we sort all the model names alphabetically for printing.
• in pytest, we can test every model that has prefix NN instead of hard-coding their names in our test cases.

[kallewesterling]

I like this suggestion - what do you think @mastoffel?

[mastoffel]

both good suggestions! I'm on it.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 94.77612% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 90.31%. Comparing base (69932f1) to head (8fedada).

Files	Patch %	Lines
autoemulate/compare.py	85.71%	2 Missing ⚠️
autoemulate/plotting.py	33.33%	2 Missing ⚠️
autoemulate/hyperparam_searching.py	0.00%	1 Missing ⚠️
autoemulate/save.py	87.50%	1 Missing ⚠️
autoemulate/utils.py	87.50%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #196      +/-   ##
==========================================
+ Coverage   88.09%   90.31%   +2.21%     
==========================================
  Files          44       44              
  Lines        2083     2085       +2     
==========================================
+ Hits         1835     1883      +48     
+ Misses        248      202      -46     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kallewesterling]

LGTM.

Have one suggestion for NeuralNetTorch model names.

Also, do you think it would be useful to have a getter function / attribute for all emulators that return the model name? e.g.

> model = NeuralNetTorch(module='mlp')
> model.model_name
NNMLP
> model = RandomForest()
> model.model_name 
RandomForest

This would be easy to implement through accessing <Class>.__class__.__name__ in a simple implementation like this:

@property
def model_name(self):
    return self.__class__.__name__

...at least I think...?

[bryanlimy]

LGTM.
Have one suggestion for NeuralNetTorch model names.
Also, do you think it would be useful to have a getter function / attribute for all emulators that return the model name? e.g.

> model = NeuralNetTorch(module='mlp')
> model.model_name
NNMLP
> model = RandomForest()
> model.model_name 
RandomForest

This would be easy to implement through accessing <Class>.__class__.__name__ in a simple implementation like this:

@property
def model_name(self):
    return self.__class__.__name__

...at least I think...?

NeuralNetTorch(module='mlp') and NeuralNetTorch(module='rbf') would have the same name in this case. We can have prefix + module like model_name = f"NN{upper(self.module)}" for all NeuralNetTorch architectures, which will lead to NNMLP, NNRBF etc.

[kallewesterling]

Sounds like a good solution to me!

[mastoffel]

Ok, implemented both of @bryanlimy's suggestions.

• Each model now has a model_name property method, and the get_model_name function from utils now retrieves the model name even if the model is wrapped in a Pipeline and Multioutput.
• Torch model names are now: def model_name(self): return f"NN{self.module_name.capitalize()}"
• did a few other renames to make things cleaner

[mastoffel]

Maybe would be good if you could quickly have an eye on this too @bryanlimy ! Lots of changes here, but I think it's mostly fine.

[bryanlimy]

LGTM!