search

scipion-em / scipion-em-sphire

Plugin to use Sphire programs within the Scipion framework
GNU General Public License v3.0
0 stars 1 forks source link

Refactor and 1.3.2 #3

Closed the-best-elephant closed 5 years ago

the-best-elephant commented 5 years ago
pconesa commented 5 years ago

I always thought it was part of the scipion phylosophy. I liked the idea to keep them for debug purposes. But I can see the burden of keeping all the metadata files.

El mar., 28 may. 2019 11:59, Jose Miguel de la Rosa Trevin < notifications@github.com> escribió:

@delarosatrevin commented on this pull request.

In sphire/protocols/protocol_cryolo_picking.py https://github.com/scipion-em/scipion-em-sphire/pull/3#discussion_r288023046 :

@@ -251,7 +251,7 @@ def getInputModel(self): return os.path.abspath(m) if m else ''

 def getOutputDir(self):
  • return self._getTmpPath('outputEMAN')
  • return self._getExtraPath('Boxfiles')

It is a temporary output that is parsed and no longer needed. I'm going the other way around in Relion and other plugins...removing files not needed.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/scipion-em/scipion-em-sphire/pull/3?email_source=notifications&email_token=AAF7ZYOCUBQWZIG7RLLI3ITPXT7BTA5CNFSM4HP4A322YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBZ2UDVI#discussion_r288023046, or mute the thread https://github.com/notifications/unsubscribe-auth/AAF7ZYNX7AH3D3KXHBPOBF3PXT7BTANCNFSM4HP4A32Q .

delarosatrevin commented 5 years ago

Yes, we have the philosophy of keeping original files...but in some case we generate more files than the job in the original package (e.g when using batches). So we need to be extra careful with additional files and the garbage that is being accumulated. It is no good for parallel filesystems or for moving data around.

For debugging purposes you can always activate the environment variable SCIPION_DEBUG_NOCLEAN=1 and the tmp folder will not be deleted. ;)

delarosatrevin commented 5 years ago

@pconesa , @the-best-elephant what is the current status of this PR? Do you still want to keep temporary files? Moreover, why do you commented out the training parameter in the training protocol?

pconesa commented 5 years ago

Forgot about this.....I still think is a good idea to keep metadata files but if reducing I/O become that important I'm ok to leave working dir on tmp.

On the training param...is there a point to have a pointer to another trainning in the trainning. We thought it was a legacy from a copy paste from the picking.

@the-best-elephant , why you've closed the PR?

delarosatrevin commented 5 years ago

@pconesa Even in training, it is possible to "fine tune" an existing model, that could be the general model or another one...so I think this option make sense. I'm not sure if it is properly implemented.

BTW, I'm have tested (not this branch) with crYOLO 1.3.6 (lastest one at this moment) and seems to be working.

pconesa commented 5 years ago

Yep, we missunderstood it. We can leave it closed.

the-best-elephant commented 5 years ago

@delarosatrevin, @pconesa My bad. I will test the implementation.