Logfile #31
Logfile #31
Conversation
|
Can you remove all the files inside |
…ry-files Antoniahuber delete unnecessary files
wakis/gridFIT3D.py
Outdated
| self.logger.grid_logs["dz"] = self.dz | ||
| self.logger.grid_logs["stl_solids"] = list(self.stl_solids.values()) if self.stl_solids is not None else [] | ||
| self.logger.grid_logs["stl_materials"] = list(self.stl_materials.values()) if self.stl_materials is not None else [] | ||
| if stl_rotate != [0., 0., 0.]: |
There was a problem hiding this comment.
very nice, I really like this :) helps keep the logger short
|
|
||
| class Logger(): | ||
|
|
||
| def __init__(self): |
There was a problem hiding this comment.
to stick to camelCase convention, we should avoid using underscores _. Do you think it would be still legible if we remove the _logs? like this we keep the variable format a bit more uniform. In the end, you call it using logger.wakeSolver so the log is implicit? What do you think?
There was a problem hiding this comment.
Yes I think that is fine. Should I remove them?
wakis/solverFIT3D.py
Outdated
|
|
||
| if verbose: print(f'Total initialization time: {time.time() - t0} s') | ||
|
|
||
| # assign logs |
There was a problem hiding this comment.
Okay, we're almost there!!!
I was thinking, in order to keep the (already long) _init__ short and legible, should we populate/assign the logs in a separate method? I was thinking something private like _assign_logs(self) where we can allocate all this variables since they are already contained in self.
There was a problem hiding this comment.
Yes that makes the code clearer. I will do it
There was a problem hiding this comment.
If I do that I see a problem with self.bc_low and self.bc_high. They are both changed if use_mpi is True: if self.use_mpi and self.grid.use_mpi:
if self.rank > 0:
self.bc_low=['pec', 'pec', 'mpi'] In that case you can't comprehend, what the z boundary condition was originally, can you? And if you called the assign_log function before, you wouldn't get the initialization time of the solver
wakis/wakeSolver.py
Outdated
| # create log | ||
| if self.log: | ||
| self.params_to_log() | ||
|
|
There was a problem hiding this comment.
same comment as in SolverFIT3D
Also, should we remove the self.params_to_log()?
| self.mark_cells_in_stl() | ||
| if stl_colors is None: | ||
| self.assign_colors() | ||
|
|
There was a problem hiding this comment.
Same comment as in SolverFIT3D
wakis/solverFIT3D.py
Outdated
| # Grid | ||
| self.grid = grid | ||
|
|
||
| bgLog = bg |
There was a problem hiding this comment.
not sure I like this one :( we can simply add it to self as self.bg or maybe even better self.background--I should have called background since the beginning...
There was a problem hiding this comment.
Wow, I thought there was already a self.bg, but apparently there is none. Then I prefer the self.background myself
| ti = self.ti | ||
|
|
||
| # longitudinal variables | ||
| if self.zf is None: self.zf = self.z |
There was a problem hiding this comment.
The changes for the non-uniform grid we should put in a different PR for clarity :)
| """ | ||
| for atr in attrs: | ||
| if atr == 'grid': | ||
| self.logger.grid = self.grid.logger.grid |
There was a problem hiding this comment.
grid is very heavy on memory, which information are we saving there?
There was a problem hiding this comment.
We are giving the dictionary 'logger.grid' further to the logger object of the solver.
We are saving all the parameters assigned to the grid in Grid3DFit. It is whether mesh refinement is used, Nx, Ny, Nz, dx, dy, dz, stl_solids, stl_materials, stl_rotate, stl_translate, stl_scale and the gridInitializationTime
There was a problem hiding this comment.
I see! You are just checking that, if the attribute is named "grid", then you pass only the logger and not the full grid class. Understood. Thanks!
2 participants
📝 Pull Request Summary
A new class "Logger" that collects the parameters from the wake, the grid and the solver and writes them into a .log file.
🔧 Changes Made
✅ Checklist
README.md,release.md, etc.)📌 Related Issues / PRs
Closes #29