Rckirby/getavec #6
Conversation
Added the Torchfire's residual function to the ML framework and built a demo with name heat_eq_conductivity.py.
There was a problem hiding this comment.
There is a lot of good work here.
Generally,
i) we need to run flake8 over all the Python files for cleanup purposes (e.g. white space, commented-out code, etc)
ii) I think having so many fenics->firedrake conversions in place is not a good look for us. We should eventually support FEniCS, but how hard is it to make all of the examples work end-to-end in Firedrake, or at least encapsulate the conversion (which is only used for getting kappas, I think) inside some function that we call so that every test isn't littered with loading a bunch of fenics matrices and doing ugliness.
iii) see particular comments inline in the code.
demos/heat_eq_conductivity/computing_time_heat_eq_conductivity.py
Outdated
Show resolved
Hide resolved
| neurons = 1000 | ||
|
|
||
|
|
||
| # ! 1. Loading data by pandas |
There was a problem hiding this comment.
This is a demo -- say in some comments why you're loading all this from pandas -- what are you doing?
There was a problem hiding this comment.
Explanations have not been added yet in similar codes. Will keep open
There was a problem hiding this comment.
@nguyenvanhaibk92 I think you are the best person to say why we are loading all the csv files. Can you add a few comments explaining this?
There was a problem hiding this comment.
At the moment, I generate data for training and testing (model inference) and save them to .csv files. Then, we load data from these files to train the neural network. Hence, we do not need to regenerate data again each time we run the code.
Pandas is just a convenient package to load data from .csv files.
There was a problem hiding this comment.
Yes, pandas is convenient. One needs to decide whether the cost of generating the data is significant relative to the cost of training?
There was a problem hiding this comment.
I added a quick comment to add what @nguyenvanhaibk92 explained. Shall I mark this as resolved?
There was a problem hiding this comment.
Generating data time takes a few minutes, which is not much compared to training time.
The best option is to provide a python file (I do not have a "good" one for this problem, which uses a random seed that allows the users to be able to reproduce results. However, in the problem setting (in the proposal, or paper later), we have an explanation of what data (e.g. z coefficients, eigenvectors, eigenvalues...) are. I think it's fine at the moment if we do not have one.
It will take time to create one, we will have it in the next update.
demos/heat_eq_conductivity/computing_time_heat_eq_conductivity.py
Outdated
Show resolved
Hide resolved
demos/heat_eq_conductivity/computing_time_heat_eq_conductivity.py
Outdated
Show resolved
Hide resolved
demos/heat_eq_conductivity/computing_time_heat_eq_conductivity.py
Outdated
Show resolved
Hide resolved
| neurons = 1000 | ||
|
|
||
|
|
||
| # STEP 1. Loading data from .csv files |
There was a problem hiding this comment.
Add a comment as to what you're doing here (is this where the FEniCS-converted data comes from so that FEniCS is hidden from view?
There was a problem hiding this comment.
The kappa samples are drawn from KL expansion formula. Then, the solution state is obtained by Firedrake solver. There is no Fenics.
| neurons = 1000 | ||
|
|
||
|
|
||
| # STEP 1. Loading data from .csv files |
There was a problem hiding this comment.
Again, a comment is helpful, here and later in CSV?
There was a problem hiding this comment.
Adding more comments to all files nFEM and TNet
Do not merge until results from @nguyenvanhaibk92 have been added. PR open so we can start working on any fix that may be done at the same time