-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
task/WA-155: Potree v3 apps #34
base: main
Are you sure you want to change the base?
Conversation
"description": "", | ||
"dynamicExecSystem": false, | ||
"execSystemConstraints": null, | ||
"execSystemId": "wma-dcv-01", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if using wma-dcv-01
could be a problem as it has 32GB of memory. Some large las files could use 50gb of memory (we have have hit this in hazmapper so our backend worker has 100 GB of memory and we have plans to limit the amount of simulation conversion of large las files).
I'm not certain if this would be a problem at all here. but something to consider I guess for all apps that share wma-dcv-01. maybe, we just need to ask for more memory on that machine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That said, this looks to be a more recent version fo Potree Converter so I'm not certain on the memory requirements
|
||
echo "Potree Converter 2.1.1" | ||
|
||
/opt/PotreeConverter/build/PotreeConverter ${converterInput} -o ${_tapisExecSystemOutputDir} -m ${samplingMethod} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would this need --generate-page index
? so that it can be viewed as a web page in the viewer app?
-p [ --generate-page ] Generate a ready to use web page with the given name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously, there was an extra arguments input field? should that be done instead of the samplingMethod
(unless samplingMethod was a requirement from somebody). and if we do that we can improve things by always adding --generate-page index
and then add any extra. then we could avoid having all the UI text about "To use the PotreeViewer App, you must add -p index in the arguments".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, completely agreed, had to dig a bit more into their code to find the command options - I will update with a reference to their commandline args. Also agreed on keeping the generate-page
arg in there by default.
Overview
Convert Potree Converter & Potree Viewer DS apps to TAPIS v3
Includes
WA-155
Changes
Testing
potree/potree-converter
for potree converter)Notes