Fix adaptive refinement#571
Conversation
5e9ccc6 to
ca57ef0
Compare
Code Coverage SummaryResults for commit: 2a316a8 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes issue #539 by refactoring the adaptive refinement functionality and updating the related tests and documentation. Key changes include the migration of the R3Refinement callback to a new module structure, the removal of the deprecated adaptive_refinement_callback, and an addition of a dataset update method to support the new refinement logic.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_callback/test_adaptive_refinement_callback.py | Updated imports and test functions for the new R3Refinement callback, including a keyword change from "callback" to "callbacks". |
| pina/data/dataset.py | Added update_data() method to enable dataset updates with new conditions. |
| pina/callback/refinement/refinement_interface.py | Introduced the new refinement interface with adaptive point updating logic. |
| pina/callback/refinement/r3_refinement.py | Implemented the R3Refinement callback for adaptive point sampling. |
| pina/callback/adaptive_refinement_callback.py | Removed the deprecated adaptive refinement callback implementation. |
| pina/callback/init.py | Updated callback exports to reflect the new module structure. |
| docs | Revised documentation to reference updated module paths for refinement callbacks. |
|
@dario-coscia I fixed codacy warnings in your code! Let me know if the changes are ok! |
Great! Code-wise and test, I think we are ready to go. Would you mind adjusting the docstrings ? |
|
@dario-coscia Done! |
|
For me is good to go, @GiovanniCanali what do you think? |
GiovanniCanali
left a comment
There was a problem hiding this comment.
Very nice update! I left few comments to address before merging.
| parameter. | ||
|
|
||
| :param dict new_conditions_dict: Dictionary containing the new data. | ||
| :type new_conditions_dict: dict |
There was a problem hiding this comment.
Please, remove :type new_conditions_dict: dict, as type is already specified in the previous line.
| :param ~lightning.pytorch.trainer.trainer.Trainer: The trainer object. | ||
| :param PINNInterface solver: The solver object. | ||
| """ | ||
| if trainer.current_epoch % self.sample_every == 0: |
There was a problem hiding this comment.
I would modify this control as follows:
if (trainer.current_epoch % self.sample_every == 0) and (trainer.current_epoch != 0)This is done to avoid resampling after the first epoch.
| :param solver: The solver object. | ||
| :return: New points sampled based on the R3 strategy. | ||
| :rtype: LabelTensor | ||
| """ |
There was a problem hiding this comment.
Please, add parameters' type to this docstring.
* Fix adaptive refinement (#571) --------- Co-authored-by: Dario Coscia <93731561+dario-coscia@users.noreply.github.com> * Remove collector * Fixes * Fixes * rm unnecessary comment * fix advection (#581) * Fix tutorial .html link (#580) * fix problem data collection for v0.1 (#584) * Message Passing Module (#516) * add deep tensor network block * add interaction network block * add radial field network block * add schnet block * add equivariant network block * fix + tests + doc files * fix egnn + equivariance/invariance tests Co-authored-by: Dario Coscia <dariocos99@gmail.com> --------- Co-authored-by: giovanni <giovanni.canali98@yahoo.it> Co-authored-by: AleDinve <giuseppealessio.d@student.unisi.it> * add type checker (#527) --------- Co-authored-by: Filippo Olivo <filippo@filippoolivo.com> Co-authored-by: Giovanni Canali <115086358+GiovanniCanali@users.noreply.github.com> Co-authored-by: giovanni <giovanni.canali98@yahoo.it> Co-authored-by: AleDinve <giuseppealessio.d@student.unisi.it>
* Fix adaptive refinement (mathLab#571) --------- Co-authored-by: Dario Coscia <93731561+dario-coscia@users.noreply.github.com> * Remove collector * Fixes * Fixes * rm unnecessary comment * fix advection (mathLab#581) * Fix tutorial .html link (mathLab#580) * fix problem data collection for v0.1 (mathLab#584) * Message Passing Module (mathLab#516) * add deep tensor network block * add interaction network block * add radial field network block * add schnet block * add equivariant network block * fix + tests + doc files * fix egnn + equivariance/invariance tests Co-authored-by: Dario Coscia <dariocos99@gmail.com> --------- Co-authored-by: giovanni <giovanni.canali98@yahoo.it> Co-authored-by: AleDinve <giuseppealessio.d@student.unisi.it> * add type checker (mathLab#527) --------- Co-authored-by: Filippo Olivo <filippo@filippoolivo.com> Co-authored-by: Giovanni Canali <115086358+GiovanniCanali@users.noreply.github.com> Co-authored-by: giovanni <giovanni.canali98@yahoo.it> Co-authored-by: AleDinve <giuseppealessio.d@student.unisi.it>
* Fix adaptive refinement (mathLab#571) --------- Co-authored-by: Dario Coscia <93731561+dario-coscia@users.noreply.github.com> * Remove collector * Fixes * Fixes * rm unnecessary comment * fix advection (mathLab#581) * Fix tutorial .html link (mathLab#580) * fix problem data collection for v0.1 (mathLab#584) * Message Passing Module (mathLab#516) * add deep tensor network block * add interaction network block * add radial field network block * add schnet block * add equivariant network block * fix + tests + doc files * fix egnn + equivariance/invariance tests Co-authored-by: Dario Coscia <dariocos99@gmail.com> --------- Co-authored-by: giovanni <giovanni.canali98@yahoo.it> Co-authored-by: AleDinve <giuseppealessio.d@student.unisi.it> * add type checker (mathLab#527) --------- Co-authored-by: Filippo Olivo <filippo@filippoolivo.com> Co-authored-by: Giovanni Canali <115086358+GiovanniCanali@users.noreply.github.com> Co-authored-by: giovanni <giovanni.canali98@yahoo.it> Co-authored-by: AleDinve <giuseppealessio.d@student.unisi.it>
* Fix adaptive refinement (mathLab#571) --------- Co-authored-by: Dario Coscia <93731561+dario-coscia@users.noreply.github.com> * Remove collector * Fixes * Fixes * rm unnecessary comment * fix advection (mathLab#581) * Fix tutorial .html link (mathLab#580) * fix problem data collection for v0.1 (mathLab#584) * Message Passing Module (mathLab#516) * add deep tensor network block * add interaction network block * add radial field network block * add schnet block * add equivariant network block * fix + tests + doc files * fix egnn + equivariance/invariance tests Co-authored-by: Dario Coscia <dariocos99@gmail.com> --------- Co-authored-by: giovanni <giovanni.canali98@yahoo.it> Co-authored-by: AleDinve <giuseppealessio.d@student.unisi.it> * add type checker (mathLab#527) --------- Co-authored-by: Filippo Olivo <filippo@filippoolivo.com> Co-authored-by: Giovanni Canali <115086358+GiovanniCanali@users.noreply.github.com> Co-authored-by: giovanni <giovanni.canali98@yahoo.it> Co-authored-by: AleDinve <giuseppealessio.d@student.unisi.it>
Description
This PR fixes #539
Checklist